Skip to content

Fix for duplicate coordinates in bbox_lonlats for geostationary area #475

Merged
djhoese merged 4 commits into
pytroll:mainfrom
ghiggi:bugfix-duplicate-geo-bbox-lonlats
Nov 17, 2022
Merged

Fix for duplicate coordinates in bbox_lonlats for geostationary area #475
djhoese merged 4 commits into
pytroll:mainfrom
ghiggi:bugfix-duplicate-geo-bbox-lonlats

Conversation

@ghiggi

@ghiggi ghiggi commented Nov 15, 2022

Copy link
Copy Markdown
Contributor

This PR removes the presence of duplicate consecutive coordinates in the geostationary are bbox/polygon.
This currently leads to the creation of SArcs that are actually points, and also leads to cross-product operations that lead to cartesian [0,0,0] and thus vector norm = 0 and thus division by 0 when normalizing.

Furthermore, it closes #474: when we retrieve 4 vertices we now get the 4 correct geos_area "extent" coordinates.

@codecov

codecov Bot commented Nov 15, 2022

Copy link
Copy Markdown

Codecov Report

Merging #475 (d22496f) into main (1268d90) will decrease coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #475      +/-   ##
==========================================
- Coverage   94.28%   94.24%   -0.05%     
==========================================
  Files          69       73       +4     
  Lines       12394    12703     +309     
==========================================
+ Hits        11686    11972     +286     
- Misses        708      731      +23     
Flag Coverage Δ
unittests 94.24% <100.00%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pyresample/geometry.py 87.17% <100.00%> (ø)
pyresample/test/test_geometry.py 99.51% <100.00%> (ø)
pyresample/spherical.py 92.67% <0.00%> (-5.55%) ⬇️
pyresample/test/test_spherical.py 100.00% <0.00%> (ø)
pyresample/future/spherical/point.py 100.00% <0.00%> (ø)
pyresample/future/spherical/__init__.py 100.00% <0.00%> (ø)
pyresample/test/test_sgeom/__init__.py 100.00% <0.00%> (ø)
pyresample/test/test_sgeom/test_point.py 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@coveralls

coveralls commented Nov 15, 2022

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.03%) to 93.761% when pulling d22496f on ghiggi:bugfix-duplicate-geo-bbox-lonlats into 1268d90 on pytroll:main.

@djhoese djhoese added the bug label Nov 17, 2022
Comment thread pyresample/geometry.py Outdated
Comment thread pyresample/geometry.py Outdated
@djhoese djhoese self-assigned this Nov 17, 2022

@djhoese djhoese left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this makes sense to the extent (pun intended) that I understand it. The solution seems simple enough. Thanks for fixing this. I'll merge it once tests pass and you can merge it into your boundary PR #473 and then we can merge that.

@djhoese

djhoese commented Nov 17, 2022

Copy link
Copy Markdown
Member

Some of the uncovered lines that coveralls is complaining about are a little concerning, but I assume you'll be working with this in the upcoming PRs so I'm not too worried. Most of the missed lines are the matplotlib plotting code which is annoying to test anyway. The one concerning one is the vertices to xyz helper function. The branch with the .T is reported as uncovered. Let's merge this for now and have you work on covering that line in one of the other PRs.

@djhoese djhoese merged commit ff0e843 into pytroll:main Nov 17, 2022
@ghiggi ghiggi deleted the bugfix-duplicate-geo-bbox-lonlats branch November 18, 2022 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

get_geostationary_bounding_box* contains duplicated vertices at the equator

3 participants