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

Extension to the HDF5 chunks API #310

Merged
merged 41 commits into from
Aug 31, 2024
Merged

Extension to the HDF5 chunks API #310

merged 41 commits into from
Aug 31, 2024

Conversation

davidhassell
Copy link
Contributor

@davidhassell davidhassell commented Aug 6, 2024 •
edited
Loading

Fixes #309

Don't be put off by the number of files changed! A fair amount of propagation of common themes ...)

@davidhassell davidhassell added enhancement New feature or request performance Relating to speed and memory performance netCDF write Relating to writing netCDF datasets netCDF read Relating to reading netCDF datasets labels Aug 6, 2024
@davidhassell
Copy link
Contributor Author

Hi Sadie - not that urgent at the moment, I think - no need to drop things for this!

sadielbartholomew
sadielbartholomew approved these changes Aug 27, 2024
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.

Implementation and functionality are all good, with sufficient new testing and docs via docstring updates. I have made some minor comments, but overall happy for you to merge when ready. Thanks.

cfdm/data/data.py Show resolved Hide resolved
cfdm/docstring/docstring.py Show resolved Hide resolved
cfdm/docstring/docstring.py Outdated Show resolved Hide resolved
cfdm/docstring/docstring.py Outdated Show resolved Hide resolved
cfdm/docstring/docstring.py Outdated Show resolved Hide resolved
cfdm/read_write/write.py Outdated Show resolved Hide resolved
cfdm/read_write/write.py Outdated Show resolved Hide resolved
cfdm/mixin/netcdf.py Outdated Show resolved Hide resolved
cfdm/read_write/netcdf/netcdfwrite.py Outdated Show resolved Hide resolved
cfdm/read_write/netcdf/netcdfwrite.py Outdated Show resolved Hide resolved
davidhassell and others added 15 commits August 28, 2024 07:49
@davidhassell @sadielbartholomew
Typos
296f62a 
Co-authored-by: Sadie L. Bartholomew <[email protected]>
@davidhassell
hdf5 dhunks sequence order docs
6ee677f
@davidhassell
ints only in hdf_chunksize sequence
4f3e123
@davidhassell
improve HDF5 chunks docs
699aad8
@davidhassell
improve HDF5 chunks docs: dict input
4437ed6
@davidhassell
improve HDF5 chunks docs: dict input (2)
4ec1205
@davidhassell @sadielbartholomew
Improve docs wording
f6f0f58 
Co-authored-by: Sadie L. Bartholomew <[email protected]>
@davidhassell
new method NetCDFWrite._compressed_data
c16d212
@davidhassell
Merge branch 'hdf5-chunks' of github.com:davidhassell/cfdm into hdf5-…
ac4d567 
…chunks
@davidhassell @sadielbartholomew
Include default value in docstring
588528a 
Co-authored-by: Sadie L. Bartholomew <[email protected]>
@davidhassell @sadielbartholomew
Update docs/source/introduction.rst
ee63331 
Co-authored-by: Sadie L. Bartholomew <[email protected]>
@davidhassell @sadielbartholomew
Typo
6c9a891 
Co-authored-by: Sadie L. Bartholomew <[email protected]>
@davidhassell @sadielbartholomew
Typo
04d1161 
Co-authored-by: Sadie L. Bartholomew <[email protected]>
@davidhassell @sadielbartholomew
Typo
052ef6e 
Co-authored-by: Sadie L. Bartholomew <[email protected]>
@davidhassell
typo
2f8ebc1
@davidhassell
Copy link
Contributor Author

Hi Sadie - thanks for the deep review. I've responded to all of your points - let me know if you're happy with all of it.

sadielbartholomew
sadielbartholomew approved these changes Aug 28, 2024
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.

Thanks for the clarifications and minor improvements after my feedback, all addressed very well. There are a few typos introduced in the updates then it's ready to merge in my view, so please go ahead.

cfdm/read_write/netcdf/netcdfwrite.py Outdated Show resolved Hide resolved
cfdm/read_write/netcdf/netcdfwrite.py Outdated Show resolved Hide resolved
cfdm/read_write/netcdf/netcdfwrite.py Outdated Show resolved Hide resolved
davidhassell and others added 2 commits August 31, 2024 12:17
@davidhassell @sadielbartholomew
Typo
a606721 
Co-authored-by: Sadie L. Bartholomew <[email protected]>
@davidhassell @sadielbartholomew
Typo
beb4e0b 
Co-authored-by: Sadie L. Bartholomew <[email protected]>
@davidhassell davidhassell merged commit 8d3b7d2 into NCAS-CMS:main Aug 31, 2024
@davidhassell davidhassell deleted the hdf5-chunks branch August 31, 2024 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sadielbartholomew sadielbartholomew sadielbartholomew approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request netCDF read Relating to reading netCDF datasets netCDF write Relating to writing netCDF datasets performance Relating to speed and memory performance
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Extension to the HDF5 chunks API
2 participants
@davidhassell @sadielbartholomew