Skip to content

use vertices_per_side argument rather than frequency which is deprecated#712

Merged
mraspaud merged 2 commits intopytroll:mainfrom
jensdebruijn:main
Mar 12, 2026
Merged

use vertices_per_side argument rather than frequency which is deprecated#712
mraspaud merged 2 commits intopytroll:mainfrom
jensdebruijn:main

Conversation

@jensdebruijn
Copy link
Contributor

@jensdebruijn jensdebruijn commented Mar 9, 2026

get_polygon_to_contain used the deprecated frequency argument:

The get_polygon_to_contain function and call to get_edge_bbox_in_projection_coordinates:

x, y = self.area_to_contain.get_edge_bbox_in_projection_coordinates(frequency=10)

And the "PendingDeprecationWarning" in get_edge_bbox_in_projection_coordinates:

warnings.warn("The `frequency` argument is pending deprecation, use `vertices_per_side` instead",

@codecov
Copy link

codecov bot commented Mar 9, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.67%. Comparing base (4da28b0) to head (7ef8821).
⚠️ Report is 37 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #712   +/-   ##
=======================================
  Coverage   93.67%   93.67%           
=======================================
  Files          89       89           
  Lines       13621    13656   +35     
=======================================
+ Hits        12759    12792   +33     
- Misses        862      864    +2     
Flag Coverage Δ
unittests 93.67% <100.00%> (+<0.01%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jensdebruijn
Copy link
Contributor Author

Found a few more instances of the same issue. Also fixed now.

Copy link
Member

@djhoese djhoese left a comment

Choose a reason for hiding this comment

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

If I remember correctly @mraspaud made the original switch to the newer vertices_per_side kwarg. @mraspaud could you verify that the changes made here make sense? Do you have any memory why they weren't changed when vertices_per_side was added originally?

@mraspaud
Copy link
Member

IIRC, a frequency=10 meant a vertex every 10 pixels, so just using 10 for vertices_per_sides will probably yield different results...

@jensdebruijn
Copy link
Contributor Author

jensdebruijn commented Mar 12, 2026

I did check the functions and vertices_per_sides and frequency are interpreted identically in the called functions (here and here). So as far as I understand, this change wouldn't change behaviour at all, only stop the warning.

My knowledge of pyresample is quite limited, but what I understand from your comment is that perhaps they should be interpreted differently?

@mraspaud
Copy link
Member

Indeed, my bad, thanks for pointing me to the implementation!
I don't think they need to be interpreted differently, the new name is actually clearing a confusion.

Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for reducing the warning count!

@mraspaud mraspaud merged commit 3b10d51 into pytroll:main Mar 12, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

get_polygon_to_contain uses the deprecated frequency argument

3 participants