Skip to content

Conversation

@alinelena
Copy link
Member

No description provided.

Copy link
Member

@ElliottKasoar ElliottKasoar left a comment

Choose a reason for hiding this comment

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

Generally looks good, thanks!

We should add a test for the hdf5 bands, and it looks like there are a couple of LZMAErrors in the tests still.

@ElliottKasoar ElliottKasoar added the enhancement New/improved feature or request label Jul 24, 2025
oerc0122
oerc0122 previously approved these changes Aug 12, 2025
Copy link
Collaborator

@oerc0122 oerc0122 left a comment

Choose a reason for hiding this comment

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

Looks ok codewise when it works, just one small potential clarification.

Copy link
Member

@ElliottKasoar ElliottKasoar left a comment

Choose a reason for hiding this comment

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

Generally looks great, thanks!

Should we perhaps add a quick check that the new hdf5 can be loaded in at least one of the tests? All the places where we used to open the .xz file are now just yaml, so I don't think we test it anywhere.

@ElliottKasoar ElliottKasoar merged commit 2345cc7 into stfc:main Sep 30, 2025
13 checks passed
@ElliottKasoar ElliottKasoar added the breaking API breaking changes label Sep 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking API breaking changes enhancement New/improved feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants