Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor boundary creation logic #525

Open
djhoese opened this issue Jun 21, 2023 · 0 comments
Open

Refactor boundary creation logic #525

djhoese opened this issue Jun 21, 2023 · 0 comments
Labels
backwards-incompatibility Causes backwards incompatibility or introduces a deprecation enhancement refactor

Comments

@djhoese
Copy link
Member

djhoese commented Jun 21, 2023

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.

@djhoese djhoese added enhancement refactor backwards-incompatibility Causes backwards incompatibility or introduces a deprecation labels Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backwards-incompatibility Causes backwards incompatibility or introduces a deprecation enhancement refactor
Projects
None yet
Development

No branches or pull requests

1 participant