opendatacube / odc-geo

GeoBox and geometry utilities extracted from datacube-core
https://odc-geo.readthedocs.io/en/latest/
Apache License 2.0
83 stars 15 forks source link

Unify reproject behaviour on Dataset and DataArray #124

Closed Kirill888 closed 9 months ago

Kirill888 commented 9 months ago

Expose all the possible GeoBox.from_bbox parameters on .reproject, .output_geobox, .to_crs

github-actions[bot] commented 9 months ago

🚀 Deployed on https://65bb3324e18bb4353b2acb18--odc-geo-docs.netlify.app

codecov[bot] commented 9 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (2d5abde) 95.26% compared to head (fc38cc7) 95.28%. Report is 3 commits behind head on develop.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #124 +/- ## =========================================== + Coverage 95.26% 95.28% +0.01% =========================================== Files 31 31 Lines 5280 5301 +21 =========================================== + Hits 5030 5051 +21 Misses 250 250 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

robbibt commented 9 months ago

Still reviewing this, but a small issue I've found so far: shape isn't respected if you try to reproject to the same CRS but different shape:

import rioxarray
import odc.geo.xr

ds = rioxarray.open_rasterio(
    filename="https://data.dea.ga.gov.au/derivative/ga_ls_wo_fq_cyear_3/1-6-0/x30/y24/1995--P1Y/ga_ls_wo_fq_cyear_3_x30y24_1995--P1Y_final_frequency.tif",
    band_as_variable=True,
    chunks={},
)

ds_reprojected = ds.odc.reproject(how="EPSG:3577", shape=(100, 100))

print(ds.odc.geobox.shape)
print(ds_reprojected.odc.geobox.shape)

image

robbibt commented 9 months ago

And a more general comment: I find it difficult to find out what params are actually supported by .odc.reproject(). When I try and inspect the available docstring (e.g. shift tab) I get this: image

...which doesn't provide any mention of the new GeoBox.from_bbox params that are now supported (e.g. resolution, anchor, shape etc).

Is there any way we can get a more detailed/helpful docstring to appear here (or include the new params with default values in the function definition so they are at least visible)? Trying to work out what params are supported by .odc.() extension methods is something that other users in DEA have struggled with too - having more descriptive/easy to access docs could really help improve usability here I think.

robbibt commented 9 months ago

Have finished reviewing this - everything is working really nicely. Is there an obvious solution for the shape issue above? Let me know if you're planning to fix it before merging this PR, or prefer to merge this as-is and solve that later (in which case I can approve).

Kirill888 commented 9 months ago

Have finished reviewing this - everything is working really nicely. Is there an obvious solution for the shape issue above? Let me know if you're planning to fix it before merging this PR, or prefer to merge this as-is and solve that later (in which case I can approve).

I'd rather make sure that case has a test and works first. I can merge it without approve anyway, so no issue there.

Thanks for doc suggestions.

Kirill888 commented 9 months ago

@robbibt this should all be fixed now, I'll merge now.