pytroll / pytroll-collectors

Collector modules for Pytroll
GNU General Public License v3.0
3 stars 18 forks source link

Refactor geographic gathering #85

Closed pnuu closed 3 years ago

pnuu commented 3 years ago

This PR refactors the code gatherer.py uses, deprecates gatherer.py in favor of more descriptive geographic_gatherer.py and does general clean-up of the code.

codecov[bot] commented 3 years ago

Codecov Report

Merging #85 (857b24d) into main (5429c1f) will increase coverage by 5.59%. The diff coverage is 76.54%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #85      +/-   ##
==========================================
+ Coverage   77.67%   83.26%   +5.59%     
==========================================
  Files          14       19       +5     
  Lines        2513     2624     +111     
==========================================
+ Hits         1952     2185     +233     
+ Misses        561      439     -122     
Impacted Files Coverage Δ
pytroll_collectors/file_notifiers.py 0.00% <0.00%> (ø)
pytroll_collectors/tests/__init__.py 40.00% <0.00%> (ø)
pytroll_collectors/triggers/_inotify.py 34.21% <34.21%> (ø)
pytroll_collectors/triggers/_watchdog.py 39.65% <39.65%> (ø)
pytroll_collectors/triggers/__init__.py 70.00% <70.00%> (ø)
pytroll_collectors/triggers/_base.py 74.81% <74.81%> (ø)
pytroll_collectors/region_collector.py 81.50% <77.14%> (+3.14%) :arrow_up:
pytroll_collectors/geographic_gatherer.py 83.20% <83.20%> (ø)
pytroll_collectors/triggers/_posttroll.py 90.56% <90.56%> (ø)
pytroll_collectors/helper_functions.py 66.35% <100.00%> (ø)
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5429c1f...857b24d. Read the comment docs.

pnuu commented 3 years ago

I think this has gotten far enough so that the first round of reviews are called for. I'll be continuing to comb through the code if I can spot something.

gerritholl commented 3 years ago

Could you update the documentation too?

pnuu commented 3 years ago

Will do! Forgot we had any, but you did add some a while back :grin:

gerritholl commented 3 years ago

Will I be able to use the new geographic_gatherer.py as a drop-in replacement of gatherer.py, or will users need to make any other changes to their configuration?

pnuu commented 3 years ago

It's a drop-in replacement. Basically I just renamed the script for clarity, and added a call from the old name to the new one. The major part is the internal restructuring, which shouldn't affect the usage.