feat/use new multiscales #75
Conversation
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
| "scale_factor": 2**level, | ||
| "translation_relative": 0.0, | ||
| "scale_absolute": 1.0, | ||
| "scale_relative": 2**level, |
There was a problem hiding this comment.
can we imaging other base scale factors than 2?
- Added `initialize_crs_from_dataset` function to extract CRS from dataset metadata. - Updated S2 optimization commands to include new CRS handling. - Removed unused arguments related to geometry and meteorology groups. - Added comprehensive tests for CRS initialization from various sources.
emmanuelmathot
left a comment
emmanuelmathot
left a comment
There was a problem hiding this comment.
besides my addings for a better support of CRS after CPM 2.6.0, this is great!
I validated with a test in titiler-eopf
| # verify exact output group structure | ||
| # this is a sensitive, brittle check | ||
| expected_structure_json = tuplify_json( | ||
| json.loads( | ||
| ( | ||
| Path("tests/test_data_api/geozarr_examples/") | ||
| / (s2_group_example.stem + ".json") | ||
| ).read_text() | ||
| ) | ||
|
|
||
| # Save as zarr | ||
| ds.to_zarr(test_input, zarr_format=3) | ||
| ds.close() | ||
|
|
||
| # Test CLI with --crs-groups but no groups specified | ||
| cmd = [ | ||
| ) | ||
| observed_structure_json = tuplify_json( | ||
| GroupSpec.from_zarr(zarr.open_group(output_path)).model_dump() | ||
| ) | ||
| assert expected_structure_json == observed_structure_json, view_json_diff( | ||
| expected_structure_json, observed_structure_json | ||
| ) |
There was a problem hiding this comment.
@emmanuelmathot this is an important check -- we are very strictly checking the structure of the zarr group against a reference JSON document. Any time we change the output format, this will fail, which is a sign that we need to update the JSON files that represent the expected Zarr group structure
| return | ||
| multiscales: TMSMultiscalesAttrsJSON | MultiscalesAttrsJSON | ||
|
|
||
| if multiscales_flavor == "ogc_tms": |
There was a problem hiding this comment.
Dont you think we could simply have both multiscales flavor combined until we are aligned and #77 is implemented and integrated in titiler?
There was a problem hiding this comment.
yes, I can make this kwarg a set of strings instead of as single string
| @@ -0,0 +1,16098 @@ | |||
| { | |||
There was a problem hiding this comment.
@emmanuelmathot sorry for the number of lines here but this JSON document is used to check the output of the s2 optimized conversion. that means whenever we change the output of the conversion, we need to update this document. I think we can rely on better checks in the future.
4 participants
This is an extensive, but still first-order, refactorization of the optimized s2 multiscales conversion to use the new multiscales metadata.
A few key changes:
S2OptimizedConverter,S2MultiscalePyramid, andS2OptimizationValidatorclasses have been broken out into functions.