add import rioxarray where readers actually need them #2824
Conversation
some readers use rasterio engine from xarray, but it actually use rioxarray in the backend, without stating it in the dependencies
satpy/readers/generic_image.py
Outdated
| @@ -31,6 +31,7 @@ | |||
| import dask.array as da | |||
| import numpy as np | |||
| import rasterio | |||
| import rioxarray # noqa: F401, need by xarray with the engine rasterio | |||
There was a problem hiding this comment.
This comment uses a , to separate the rule from the description, but the other one uses a :. What is the recommended way of doing this (I assume it is supported)?
There was a problem hiding this comment.
Right, the , can be used when multiple rules should be ignored. i.e. : x=5 # noqa: E731,E123 So If I want to be fully correct I should stick with some dot . or simply a space.
Moreover, as stated in the documentation :
Contents before and after the
# noqa: ...portion are ignored so multiple comments may appear on one line. Here are several examples:# mypy requires `# type: ignore` to appear first x = 5 # type: ignore # noqa: ABC123 # can use to add useful user information to a noqa comment y = 6 # noqa: ABC456 # TODO: will fix this later
pyproject.toml
Outdated
| @@ -66,7 +67,7 @@ gms5-vissr_l1b = ["numba"] | |||
| # Writers: | |||
| cf = ["h5netcdf >= 0.7.3"] | |||
| awips_tiled = ["netCDF4 >= 1.1.8"] | |||
| geotiff = ["rasterio", "trollimage[geotiff]"] | |||
| geotiff = ["rasterio", "rioxarray", "trollimage[geotiff]"] | |||
There was a problem hiding this comment.
Correct me if I'm wrong, but I'm 99% sure the geotiff writer does not use rioxarray.
There was a problem hiding this comment.
Exact, I saw rasterio so I had an urge to add rioxarray too
|
deleted rioxarray dependencies for geotiff writer, and standardized the |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2824 +/- ##
==========================================
- Coverage 95.94% 95.94% -0.01%
==========================================
Files 366 366
Lines 53613 53506 -107
==========================================
- Hits 51441 51334 -107
Misses 2172 2172
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Pull Request Test Coverage Report for Build 9561417619Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
|
Thanks @ludwigVonKoopa! |
some readers use
rasterioengine from xarray, but it actually use rioxarray in the backend, without stating it in the dependencies (see pydata/xarray#7831).It should be a xarray problems, and in the PR they tend to agree to udpate the message without stating explicitly which exact dependency is problematic (which I understand).
So importing
rioxarrayin the reader file seems the best compromise : It complain with anModuleNotFoundError, which is easier to understand why it doesn't work and how to correct that.