pytroll / pyresample

Geospatial image resampling in Python
http://pyresample.readthedocs.org
GNU Lesser General Public License v3.0
344 stars 95 forks source link

Replace and deprecate frequency arg for bbox methods #526

Closed mraspaud closed 1 year ago

mraspaud commented 1 year ago

This PR replaces the frequency argument used in many *Definition method with the more correct bbox_shape. Historically, the name was correct, but a behaviour-changing commit (19e1ea5) it became inaccurate.

codecov[bot] commented 1 year ago

Codecov Report

Merging #526 (bea2050) into main (2a6d769) will decrease coverage by 0.10%. Report is 12 commits behind head on main. The diff coverage is 95.12%.

@@            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     
Flag Coverage Δ
unittests 94.25% <95.12%> (-0.10%) :arrow_down:

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

Files Changed Coverage Δ
pyresample/geometry.py 87.63% <95.12%> (+0.04%) :arrow_up:

... and 2 files with indirect coverage changes

coveralls commented 1 year ago

Coverage Status

coverage: 93.856% (-0.1%) from 93.953% when pulling bea20503be3a8d46888cd4e5cdb007c66ae7f839 on mraspaud:fix-frequency into 2a6d7694265a13f78b2f76b4951f6851dceb30c8 on pytroll:main.

ghiggi commented 1 year ago

Hey @mraspaud :) If you manage to get this merged in the next couple of days, next week I can make the PRs for what was discussed in https://github.com/pytroll/pyresample/issues/525 ;)

mraspaud commented 1 year ago

Ok, I renamed to vertices_per_side and adjusted the docstrings to remove the parenthesis

djhoese commented 1 year ago

Coverage is reporting that the decimate method in the boundary objects is not being used anymore as well as AreaDefBoundary, but I don't see why the changes in this PR specifically are causing that. Any ideas?

https://coveralls.io/builds/61960626/source?filename=pyresample%2Fboundary.py#L106

mraspaud commented 1 year ago

I can't see anything in the PR that would change the coverage...

djhoese commented 1 year ago

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?