Skip to content

Conversation

@TAlonglong
Copy link
Collaborator

@codecov
Copy link

codecov bot commented Nov 16, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.36%. Comparing base (4f0befb) to head (3bb7cf1).
⚠️ Report is 25 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3301      +/-   ##
==========================================
+ Coverage   96.30%   96.36%   +0.05%     
==========================================
  Files         463      464       +1     
  Lines       58863    58984     +121     
==========================================
+ Hits        56689    56839     +150     
+ Misses       2174     2145      -29     
Flag Coverage Δ
behaviourtests 3.59% <0.00%> (-0.02%) ⬇️
unittests 96.45% <100.00%> (+0.05%) ⬆️

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.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@TAlonglong TAlonglong self-assigned this Nov 16, 2025
@TAlonglong
Copy link
Collaborator Author

An additional consideration is: this reader can now also read data from other scatterometers, here from the satellite EOS-06 https://space.oscar.wmo.int/satellites/view/oceansat_3_eos_06

So maybe it should be renamed; ie from scatsat1 to scatsat.

If we agree to do this renaming, how should that be done. SHould we retire the scatsat1 reader and add a scatsat reader? Or just do the renmae. Both will be breaking changes I guess.

Copy link
Member

@djhoese djhoese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love that just switching the base class to the HDF5 helper makes this work.

My biggest concern is the resolution being moved from YAML to the get_dataset method. If you do scn.available_dataset_ids() my guess is the list of datasets won't include the resolution. If that is true, then I think you'd need to define a available_datasets method on the file handler and look at the BaseFileHandler.available_datasets docstring for help on how to implement it. In that method you would add resolution to the dataset definitions based on the cell_spacing.

As for renaming and support for other satellites, I think that could be done. Would the available datasets be exactly the same?

@pnuu pnuu changed the title Issue2628 Add OSCAT-3 support to scatsat1b reader Nov 17, 2025
@TAlonglong
Copy link
Collaborator Author

pre-commit.ci autofix

@TAlonglong
Copy link
Collaborator Author

Not sure what happened with the tests now.

The old data is the same format, but 25 and 50km. I added this to reflect that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adapt scatsat1b reader to oscat-3

2 participants