Fix checking for HDF5 max dims, no longer re-create atts if not needed, confirm behavior for HDF5 cyclical files, allow user to set mpiexec #1119
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
edhartnett
changed the title
edhartnett
changed the title
|
@WardF can we get this merged soon please? I think you want this in the next release. There are some important bug fixes and the removal of an unneeded file lookup for every netcdf call, which will improve performance for power-users with thousands of files open. |
|
Double checking something on Windows, but looks good will merge shortly. |
|
Thanks! |
ckhroulev
added a commit
to ckhroulev/netcdf-c
that referenced
this pull request
Nov 13, 2019
1) We have to use H5Tequal() to compare HDF5 type IDs. 2) When checking if we can re-use an NC_CHAR attribute it is enough to compare data types (H5Tequal() takes care of the size comparison). 3) This commit adds missing code (reuse_att was set but not used). Now an attribute in a NetCDF-4 file can be modified as many times as necessary, as long its type and length remain the same. Modifications changing either type or length of an attribute require deleting and re-creating an attribute which increments the attribute order creation index. Once this index reaches 65535 all attribute modifications (for a particular group or variable) will fail. For reference: Issue 350 title: NetCDF-4 limits the number of times an attribute can be modified Pull request 1119 title: Fix checking for HDF5 max dims, no longer re-create atts if not needed, confirm behavior for HDF5 cyclical files, allow user to set mpiexec
This was referenced Nov 13, 2019
|
OK, you are quite right. Sorry about that, not sure what happened to that fix and test, but there was a lot going on in that PR - too much I guess. ;-) @WardF can you reopen this issue? I will take a look... |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #320.
Fixes #322.
Fixes #350.
Fixes #433.
Fixes #1099.
Part of #857.
Fixes #302.
This PR continues the work of preparing for lazy vars, and I also cleared up some old issues along the way.
In this PR: