pytroll / pygac

A python package to read and calibrate NOAA and Metop AVHRR GAC and LAC data
https://pygac.readthedocs.org/
GNU General Public License v3.0
20 stars 25 forks source link

Fix reading of path like objects #96

Closed oliversus closed 2 years ago

oliversus commented 2 years ago

The regular expression matching requires a string as input, not a file system object.

djhoese commented 2 years ago

So this is fixing when a Pathlib object was passed? Would it be possible for you to add a test for this?

codecov[bot] commented 2 years ago

Codecov Report

Merging #96 (2f95743) into main (93c72a9) will increase coverage by 0.04%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #96      +/-   ##
==========================================
+ Coverage   67.02%   67.06%   +0.04%     
==========================================
  Files          34       34              
  Lines        2920     2924       +4     
==========================================
+ Hits         1957     1961       +4     
  Misses        963      963              
Impacted Files Coverage Δ
pygac/reader.py 64.83% <100.00%> (+0.07%) :arrow_up:
pygac/tests/test_reader.py 99.34% <100.00%> (+<0.01%) :arrow_up:

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 93c72a9...2f95743. Read the comment docs.

oliversus commented 2 years ago

So this is fixing when a Pathlib object was passed? Would it be possible for you to add a test for this?

I'd be happy if someone else could write that test. I need this PR passed rather soon to continue processing for CMSAF. I am not that familiar with the pygac test environment and someone else would be much faster than me.

oliversus commented 2 years ago

So this is fixing when a Pathlib object was passed? Would it be possible for you to add a test for this?

I'd be happy if someone else could write that test. I need this PR passed rather soon to continue processing for CMSAF. I am not that familiar with the pygac test environment and someone else would be much faster than me.

Actually, hold on. I am trying...

sfinkens commented 2 years ago

@oliversus Thanks for the fix and adding a test, despite the urgency! The only problem was that satpy is not a dependency of pygac. I fixed that by re-using the TestPath class that already existed.

I'm not sure whether the _correct_data_set_name method is the right place for the string conversion. It may be more appropriate to do that in PODReader.read_header and KLMReader.read_header. But since there are no tests for these methods, and this fix is urgent, I would keep it as it is.