Replace and deprecate frequency arg for bbox methods #526
Conversation
Codecov Report
@@ Coverage Diff @@
## main #526 +/- ##
==========================================
- Coverage 94.34% 94.25% -0.10%
==========================================
Files 82 82
Lines 13026 13045 +19
==========================================
+ Hits 12290 12295 +5
- Misses 736 750 +14
Flags with carried forward coverage won't be shown. Click here to find out more.
|
pyresample/geometry.py
Outdated
| """Return the bounding box lons and lats. | ||
|
|
||
| Args: | ||
| frequency: | ||
| bbox_shape (formerly frequency): |
There was a problem hiding this comment.
shape? Also I don't think you can put a parenthetical in the docstring like this as sphinx will treat it like a type declaration. Also shape usually refers to a numpy shape with (row, column). Should this rename maybe reflect that this is a number of points? I got an email that @ghiggi commented the same thing, but I don't see it on here anymore. num_points?
pyresample/geometry.py
Outdated
| @@ -296,11 +296,12 @@ def get_boundary_lonlats(self): | |||
| return (SimpleBoundary(s1_lon.squeeze(), s2_lon.squeeze(), s3_lon.squeeze(), s4_lon.squeeze()), | |||
| SimpleBoundary(s1_lat.squeeze(), s2_lat.squeeze(), s3_lat.squeeze(), s4_lat.squeeze())) | |||
|
|
|||
| def get_bbox_lonlats(self, frequency: Optional[int] = None, force_clockwise: bool = True) -> tuple: | |||
| def get_bbox_lonlats(self, bbox_shape: Optional[int] = None, force_clockwise: bool = True, | |||
There was a problem hiding this comment.
Could we use n_vertices, n_vertices_per_side (preferred) or something like that? Using "shape" makes me think to an array shape ... or the output shape...
In this case the length of lons/lats is 4*bbox_shape right?
There was a problem hiding this comment.
If you switch to n_vertices_per_side I commit to do the PR next week to enable tuple arguments (of size 2 or 4) :-P
|
Ok, I renamed to |
|
Coverage is reporting that the https://coveralls.io/builds/61960626/source?filename=pyresample%2Fboundary.py#L106 |
|
I can't see anything in the PR that would change the coverage... |
|
I looked at the results again and I think there may be some confusion in the coverage reports about which commit is being reported on or something. There are coverage changes that seem relatively unrelated to what's being done here. Let's merge this and see what happens? |
This PR replaces the
frequencyargument used in many*Definitionmethod with the more correctbbox_shape.Historically, the name was correct, but a behaviour-changing commit (19e1ea5) it became inaccurate.
git diff origin/main **/*py | flake8 --diff