Refactor resampler module to submodules #3124
Conversation
pnuu
added
component:resampling
cleanup
|
The other thing about this PR that I'm realizing is that some of the resamplers (ewa maybe? kdtree nearest maybe?) exist in pyresample in a way that should be useable. Or at least that was my intention before I stopped working on pyresample 2.0. Maybe it never got all the way there (as we already know I'm wrong about the caching functionality implemented in pyresample). |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3124 +/- ##
==========================================
- Coverage 96.24% 96.23% -0.01%
==========================================
Files 421 429 +8
Lines 57671 57777 +106
==========================================
+ Hits 55506 55603 +97
- Misses 2165 2174 +9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Pull Request Test Coverage Report for Build 15308592336Details
💛 - Coveralls |
|
I think this is now ready. Unless we come up with a better name for the |
There was a problem hiding this comment.
One more comment for making things private, but also what do you think about moving get_area_file/def functions and add_xy_coords and add_crs_xy_coords to a utils.py module? Or even go as far as the get_area_* functions to one module outside of the resample subpackage (since they are used by readers and users) like satpy.area or satpy.area_utils or satpy.area_config? And add_*_coords to another module inside or outside of resampling? Like satpy.coords or satpy.coords_utils?
|
Oh another thought: if we imagine that the base resampler class(es) will all end up moving to pyresample and possibly even the "prepare_resampler" or "get_resampler"-style functions would be moved. Does that change the naming? Or rather, maybe those types of functions should not be in |
I'll have a look at this today evening after I reach the hotel in Milan, where I need to stay overnight due to a cancelled flight. |
There was a problem hiding this comment.
Awesome. I noticed one small thing that could be changed and cleaned up. The only other discussion we should maybe have is how many satpy/X_utils.py modules do we have before making things a series of utils/X.py modules? Or would it be better to drop the _utils.py part of the name and just call them satpy/area.py and satpy/coords.py?
satpy/coords_utils.py
Outdated
| except ImportError: | ||
| LOG.debug("Could not add 'crs' coordinate with pyproj<2.0") | ||
| crs = None |
There was a problem hiding this comment.
We now require pyproj>2.2. Do you want to remove this import check and either:
- Move the import to the top of the module (OK with me as I see pyproj a hard requirement of satpy).
- Make it an in-function import as it is now.
Either way, the else: block can be de-dented.
This PR refactors the
satpy/resample.pymodule to asatpy.resamplesubpackage and splits the contents to several modules.At least the warnings in the tests should be fixed, but this is the first version to see if there are other things to consider. Also some of the imports could be moved to where they are used. And the docstring should be fixed with the new import paths.
Triggered by #3123
AUTHORS.mdif not there already