Conversation
Codecov Report✅ All modified and coverable lines are covered by tests.
🚀 New features to boost your workflow:
|
TonioF
left a comment
There was a problem hiding this comment.
Looks good! Just a few minor comments, mostly typos.
| if resolution is None: | ||
| if crs is not None and crs.is_geographic: | ||
| resolution = 10 / CONVERSION_FACTOR_DEG_METER | ||
| else: | ||
| resolution = 10 |
There was a problem hiding this comment.
You have almost exactly the same code in get_native_res. You could simplify this by adapting get_native_res
There was a problem hiding this comment.
get_native_res goes from degree to meter, is crs is geographic. res_native = resolution * CONVERSION_FACTOR_DEG_METER Then after that, the nearest equal or coarser Sentinel-2 spatial resolution is selected.
The part above goes from degree to meter to degree, if crs is geographic. resolution = 10 / CONVERSION_FACTOR_DEG_METER Here, no further selection should be done.
Programmatically it would be possible, but logically it think it would complicate things.
xarray_eopf/amodes/sentinel2.py
Outdated
| crs = pyproj.CRS.from_wkt(ds.spatial_ref.attrs["crs_wkt"]) | ||
| if bbox is None: | ||
| res, ds = next(iter(datasets.items())) | ||
| crs_data = pyproj.CRS.from_wkt(ds.spatial_ref.attrs["crs_wkt"]) |
There was a problem hiding this comment.
The name 'source_crs' would have made it a lot clearer to me what is happening here.
There was a problem hiding this comment.
I will change crs_data to source_crs
Co-authored-by: Tonio Fincke <tonio.fincke@brockmann-consult.de>
Co-authored-by: Tonio Fincke <tonio.fincke@brockmann-consult.de>
Co-authored-by: Tonio Fincke <tonio.fincke@brockmann-consult.de>
crsresolutionandbbox.