-
Notifications
You must be signed in to change notification settings - Fork 295
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add reader for Landsat L1 data #2904
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2904 +/- ##
==========================================
+ Coverage 96.05% 96.11% +0.05%
==========================================
Files 370 377 +7
Lines 54334 54931 +597
==========================================
+ Hits 52191 52796 +605
+ Misses 2143 2135 -8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Pull Request Test Coverage Report for Build 11374003337Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice job, I appreciate the comprehensive tests! A few suggestions inline.
"start_time": self.date}) | ||
|
||
# Temp dir for the saved images | ||
self.base_dir = tempfile.mkdtemp() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pytest's tmp_path
is the way to go (gets cleaned up automatically also).
Also we usually have fixtures for test/synthetic files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately I don't know how fixtures work, so will be unable to add that. I'll change tmp_path
though.
For temporary files, I used pytest fixtures in the modis reader. Here, you could have something similar to:
Then, to use it in a test:
(where I hope this helps to facilitate using the fixtures if you want to go that way. It is a bit annoying at first but then I found it convenient as you can write several tests that use the sample data files with really low effort. |
Regarding the failing tests, the sza on disk contains nans sometimes :-/ I don't know enough about the geotiff writer to comment but this seems to be the root cause |
I think it's because the array I was generating ran from 0->10000. Have changed it locally to 1->10000 and haven't seen the error pop up again! Forgot that 0 was the fill value... |
…ites (based on MSI). Also, split tests into manageable chunks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR adds a reader for Landsat collection 2 level 1 data. It has been tested on Landsat 8 and 9 data and doesn't yet support older satellites in the landsat series.
Right now this is a draft as I haven't added tests, but the reader itself should be fully-functional.