Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Subsampled coordinates (1) and compression refactor #168

Merged
merged 104 commits into from
Mar 19, 2022
Merged

Subsampled coordinates (1) and compression refactor #168

merged 104 commits into from
Mar 19, 2022

Conversation

davidhassell
Copy link
Contributor

Fixes #167

@davidhassell
ideas ...
048ac66
@davidhassell
ideas ...
59b4821
@davidhassell
dev
9a92eb3
@davidhassell
dev
df5e943
@davidhassell
dev
3c426c3
@davidhassell
dev
308656f
@davidhassell
dev
acfb8fe
@davidhassell
dev
116780e
@davidhassell
dev
32f3e1d
@davidhassell
dev
2b874b7
@davidhassell
dev
ecdba8e
@davidhassell
dev
4873928
@davidhassell
dev
df439a5
@davidhassell
Merge ssh://github.com/davidhassell/cfdm into subsampled-coordinates
82ce3d6
@davidhassell
Merge branch 'subsampled-coordinates' of ssh://github.com/davidhassel…
d5aed69 
…l/cfdm into subsampled-coordinates
@davidhassell
dev
ac2be01
@davidhassell
dev
a1b9d5a
@davidhassell
dev
9dfe8e7
@davidhassell
restore netcdfwrite.py
34220e4
@davidhassell
dev
1303825
@davidhassell
Merge branch 'master' of ssh://github.com/davidhassell/cfdm into subs…
39e8479 
…ampled-coordinates
@davidhassell
dev
7120dd2
@davidhassell
all tests pass
8d57660
@davidhassell
dev
96ae8d4
@davidhassell
geographic
72e5089
@davidhassell
Merge github.com:davidhassell/cfdm into subsampled-coordinates
db31810
@davidhassell
Merge branch 'subsampled-coordinates' of github.com:davidhassell/cfdm…
94c113b 
… into subsampled-coordinates
@davidhassell
dev
f193580
@davidhassell
dev
19272fa
@davidhassell
tidy
319b86b
davidhassell and others added 2 commits March 7, 2022 14:00
@davidhassell @sadielbartholomew
Typo
d946615 
Co-authored-by: Sadie L. Bartholomew <[email protected]>
@davidhassell
Typos
ec05abe
sadielbartholomew
sadielbartholomew reviewed Mar 7, 2022
View reviewed changes
Copy link
Member

@sadielbartholomew sadielbartholomew left a comment

Choose a reason for hiding this comment

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

Final comments! All previous feedback has been addressed really well, so this is very nearly ready to merge in my eyes.

cfdm/test/test_subsampling.py Outdated
)
)

@unittest.skipIf(True, "awaiting test file")
Copy link
Member

Choose a reason for hiding this comment

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

Was this meant to be un-skipped before this PR was opened / went in (I imagine so as it's the only test in the test suite with a skip on this branch)? It guess the test needs adapting somehow (from those "change" values)...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd forgotten about this. I am still awaiting said test file from Anders and will chase it up. If it's not forthcoming I'll have to make one myself.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for clarifying. That's fine. Maybe you could either add a TODO marker there or open a GitHub Issue about this so it doesn't get forgotten about?

@davidhassell
Copy link
Contributor Author

Then I think we're done for the PR overall!

Wow. Thank you so much for your marathon review 🎆 !

Let's hold off merging in case the missing test file can be procured quickly - hopefully it won't expose something nasty ...

@davidhassell davidhassell mentioned this pull request Mar 10, 2022
Merged
@davidhassell
Copy link
Contributor Author

Still awaiting a test file for the quadratic_latitude_longitude case (units test currently skipped), but we've decided to merge anyway into master.

Master will now become v1.9.1.0b0, but v1.9.0.x lives on a branch to serve cf-python until the latter can be modified to accept subsampled coordinates (which will be the case when the move to dask has been completed: NCAS-CMS/cf-python#295)

@davidhassell davidhassell merged commit 7c32d15 into NCAS-CMS:master Mar 19, 2022
@davidhassell davidhassell added this to the 1.9.1.0 milestone Mar 21, 2022
@sadielbartholomew sadielbartholomew mentioned this pull request Mar 22, 2022
Merged
@sadielbartholomew sadielbartholomew mentioned this pull request Apr 27, 2022
Merged
@davidhassell davidhassell deleted the compression-refactor branch November 14, 2022 09:00
@davidhassell davidhassell added compression netCDF read Relating to reading netCDF datasets labels Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sadielbartholomew sadielbartholomew sadielbartholomew left review comments

Assignees
No one assigned
Labels
compression netCDF read Relating to reading netCDF datasets
Projects
None yet
Milestone
1.10.0.0
Development

Successfully merging this pull request may close these issues.

Subsampled coordinates (1) and compression refactor
2 participants
@davidhassell @sadielbartholomew