Add reader for Landsat L1 data #2904

simonrp84 · 2024-09-13T19:56:21Z

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.

Tests added
Fully documented

codecov · 2024-09-13T20:02:41Z

Codecov Report

Attention: Patch coverage is 28.22086% with 117 lines in your changes missing coverage. Please review.

Project coverage is 95.86%. Comparing base (f93e768) to head (c62eee6).
Report is 11 commits behind head on main.

Files with missing lines	Patch %	Lines
satpy/readers/oli_tirs_l1_tif.py	28.22%	117 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2904      +/-   ##
==========================================
- Coverage   96.06%   95.86%   -0.20%     
==========================================
  Files         370      372       +2     
  Lines       54320    54524     +204     
==========================================
+ Hits        52185    52272      +87     
- Misses       2135     2252     +117

Flag	Coverage Δ
behaviourtests	`3.98% <0.00%> (-0.02%)`	⬇️
unittests	`95.96% <28.22%> (-0.21%)`	⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

coveralls · 2024-09-13T20:17:48Z

Pull Request Test Coverage Report for Build 10908934386

Warning: 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.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

47 of 164 (28.66%) changed or added relevant lines in 1 file are covered.
3 unchanged lines in 1 file lost coverage.
Overall coverage decreased (-0.2%) to 95.966%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
satpy/readers/oli_tirs_l1_tif.py	47	164	28.66%

Files with Coverage Reduction	New Missed Lines	%
satpy/readers/seadas_l2.py	3	96.97%

Totals
Change from base Build 10814739875:	-0.2%
Covered Lines:	52505
Relevant Lines:	54712

💛 - Coveralls

mraspaud

Nice job, I appreciate the comprehensive tests! A few suggestions inline.

mraspaud · 2024-09-18T15:12:32Z

satpy/readers/oli_tirs_l1_tif.py

+
+    def __init__(self, filename, filename_info, filetype_info, mda, **kwargs):
+        """Initialize the reader."""
+        super(OLITIRSCHReader, self).__init__(filename, filename_info, filetype_info)


Suggested change

super(OLITIRSCHReader, self).__init__(filename, filename_info, filetype_info)

super().__init__(filename, filename_info, filetype_info)

You don't need this since python 3 :)

mraspaud · 2024-09-20T10:12:58Z

satpy/readers/oli_tirs_l1_tif.py

+            data.attrs["standard_name"] = "toa_outgoing_radiance_per_unit_wavelength"
+            data.attrs["units"] = "W m-2 um-1 sr-1"


These are provided in the yaml and made available through the info parameter (in get_dataset), would be best to use these to avoid hardcoding, right?
Same goes for the two next paragraphs...

mraspaud · 2024-09-20T10:13:44Z

satpy/readers/oli_tirs_l1_tif.py

+        return datetime(self._obs_date.year, self._obs_date.month, self._obs_date.day,
+                        self.center_time.hour, self.center_time.minute, self.center_time.second)


Do we need to add timezone info?

mraspaud · 2024-09-20T10:18:55Z

satpy/tests/reader_tests/test_oli_tirs_l1_tif.py

+class TestOLITIRSL1(unittest.TestCase):
+    """Test generic image reader."""
+
+    def setUp(self):
+        """Create temporary images and metadata to test on."""


We are using pytest mostly and dropping Testcase, so I think you can do this (not tested):

Suggested change

class TestOLITIRSL1(unittest.TestCase):

"""Test generic image reader."""

def setUp(self):

"""Create temporary images and metadata to test on."""

class TestOLITIRSL1:

"""Test generic image reader."""

def setup_method(self):

"""Create temporary images and metadata to test on."""

mraspaud · 2024-09-20T10:22:23Z

satpy/tests/reader_tests/test_oli_tirs_l1_tif.py

+                                     "start_time": self.date})
+
+        # Temp dir for the saved images
+        self.base_dir = tempfile.mkdtemp()


pytest's tmp_pathis the way to go (gets cleaned up automatically also).

Also we usually have fixtures for test/synthetic files.

Unfortunately I don't know how fixtures work, so will be unable to add that. I'll change tmp_paththough.

mraspaud · 2024-09-20T10:25:05Z

satpy/tests/reader_tests/test_oli_tirs_l1_tif.py

+        assert scn.start_time == datetime(2024, 5, 2, 18, 0, 24)
+        assert scn.end_time == datetime(2024, 5, 2, 18, 0, 24)
+
+    def test_loading(self):


Could you split up this test and others to really have one test case per function?

pdebuyl · 2024-09-25T08:54:12Z

For temporary files, I used pytest fixtures in the modis reader.

Here, you could have something similar to:

@pytest.fixture(scope="session")
def oli_tirs_l1_tif_samplefiles(tmpdir_factory) -> list[str]:
    """Create a set of OLI TIRS landsat L1 files."""
    path_1 =  str(tmpdir_factory.mktemp("oli_tirs_l1_tif").join("LC08_L1GT_026200_20240502_20240513_02_T2_MTL.xml"))
    with open(path_1, "w") as f:
      f.write(metadata_text)
    path_2 =  str(tmpdir_factory.mktemp("oli_tirs_l1_tif").join("LC08_L1GT_026200_20240502_20240513_02_T2_B4.TIF"))
    return [path_1, path_2, ...]

Then, to use it in a test:

def some_test(self, oli_tirs_l1_tif_samplefiles):
    scn = satpy.Scene(reader="oli_tirs_l1_tif", oli_tirs_l1_tif_samplefiles)

(where some_test is a member of the TestOLITIRSL1 class) will create the files on the fly.

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.

pdebuyl · 2024-09-25T09:17:35Z

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

simonrp84 added 2 commits September 9, 2024 08:14

Initial Landsat L1 commit

793fab0

Updates to Landsat L1 reader.

15a575f

simonrp84 added 8 commits September 13, 2024 22:22

Update time parser in Landsat L1 reader.

31aff4d

Fix start and end times in Landsat L1 reader.

411741c

Fix reading Landsat L1 band 9

f7e9853

Add error handling for incorrect data load.

e8fa3ac

Replace landsat area def creation technique.

c62eee6

Updates to Landsat L1 reader.

b67dc85

Add tests for Landsat L1 reader.

0054c80

Add clarification note to Landsat reader.

44ce1d2

simonrp84 marked this pull request as ready for review September 18, 2024 11:06

simonrp84 requested review from djhoese and mraspaud as code owners September 18, 2024 11:06

mraspaud reviewed Sep 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add reader for Landsat L1 data #2904

Add reader for Landsat L1 data #2904

simonrp84 commented Sep 13, 2024 •

edited

Loading

codecov bot commented Sep 13, 2024 •

edited

Loading

coveralls commented Sep 13, 2024 •

edited

Loading

mraspaud left a comment

mraspaud Sep 18, 2024

mraspaud Sep 20, 2024

mraspaud Sep 20, 2024

mraspaud Sep 20, 2024

mraspaud Sep 20, 2024

simonrp84 Sep 26, 2024

mraspaud Sep 20, 2024

pdebuyl commented Sep 25, 2024

pdebuyl commented Sep 25, 2024

	super(OLITIRSCHReader, self).__init__(filename, filename_info, filetype_info)
	super().__init__(filename, filename_info, filetype_info)

		data.attrs["standard_name"] = "toa_outgoing_radiance_per_unit_wavelength"
		data.attrs["units"] = "W m-2 um-1 sr-1"

		return datetime(self._obs_date.year, self._obs_date.month, self._obs_date.day,
		self.center_time.hour, self.center_time.minute, self.center_time.second)

Add reader for Landsat L1 data #2904

Are you sure you want to change the base?

Add reader for Landsat L1 data #2904

Conversation

simonrp84 commented Sep 13, 2024 • edited Loading

codecov bot commented Sep 13, 2024 • edited Loading

Codecov Report

coveralls commented Sep 13, 2024 • edited Loading

Pull Request Test Coverage Report for Build 10908934386

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

mraspaud left a comment

Choose a reason for hiding this comment

mraspaud Sep 18, 2024

Choose a reason for hiding this comment

mraspaud Sep 20, 2024

Choose a reason for hiding this comment

mraspaud Sep 20, 2024

Choose a reason for hiding this comment

mraspaud Sep 20, 2024

Choose a reason for hiding this comment

mraspaud Sep 20, 2024

Choose a reason for hiding this comment

simonrp84 Sep 26, 2024

Choose a reason for hiding this comment

mraspaud Sep 20, 2024

Choose a reason for hiding this comment

pdebuyl commented Sep 25, 2024

pdebuyl commented Sep 25, 2024

simonrp84 commented Sep 13, 2024 •

edited

Loading

codecov bot commented Sep 13, 2024 •

edited

Loading

coveralls commented Sep 13, 2024 •

edited

Loading