pytroll / pyresample

Geospatial image resampling in Python
http://pyresample.readthedocs.org
GNU Lesser General Public License v3.0
344 stars 95 forks source link

Refactor boundary creation logic #525

Open djhoese opened 1 year ago

djhoese commented 1 year ago

As mentioned in #524, @ghiggi had some good suggestions about how the boundary creation for geometry objects could be refactored and be made more useful:

Hey @djhoese

This looks like a good temporary fix.

I thought that in the medium-term I think would be nice to do some reorg within pyresample related to the extraction of boundary sides coordinates and boundary object creation.

For the boundary sides coordinates, we currently use get_bbox_lonlats and get_geostationary_bounding_box_in_lonlats to deal with Inf in projection coordinates of geostationary areas.

I guess that we could refactor somewhat the code in get_bbox_lonlats to:

1. call the `get_geostationary_bounding_box_in_lonlats` (when an area is geostationary) within such a method

2. and if the area is a full globe projection that has `Inf` popping up in lon/lat coordinates (after conversion from projection coordinates resulting out of Earth disk), we search within the area lons/lats array the most exterior valid coordinates. This operation would take a bit more computations (I guess all-in-memory lon/lats arrays) but would enable all downstream computations.

As a result, we could just call get_bbox_lonlats throughout the codebase.

And as a second step, we could standardize the creation of Boundary objects. Currently, we use :

* `Boundary` and `AreaDefBoundary` in `get_area_slices` and your new `_get_area_to_cover_boundary`

* `SimpleBoundary` in `get_boundary_lonlats`

* `AreaBoundary` in `boundary`.

If we would use the area.boundary() method calling the refactored get_bbox_lonlats, we would remove all the if-else logics currently reappearing many times throughout the code.

I agree with these suggestions and they make a lot of sense. This could/should maybe go in Pyresample 1.x, but I could also see it being implemented for Pyresample 2.x.