pytroll / pytroll/pyresample

Refactor boundary creation logic

Open
#525 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

backwards-incompatibility enhancement refactor
Dominant language
Python
Stars
385
Forks
102
Avg merge
4d 2h
Merged PRs (30d)
9

Description

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.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by tracing get_bbox_lonlats, get_geostationary_bounding_box_in_lonlats, get_area_slices, _get_area_to_cover_boundary, get_boundary_lonlats, and boundary. Compare their current boundary creation and handling of invalid coordinates, then define tests covering geostationary and full-globe areas; done means callers can use the standardized boundary path without repeated conditionals.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
data
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.