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.

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

check szip parameters for validity, plus all other outstanding szip work... #1629

Merged
merged 44 commits into from
Feb 12, 2020
Merged

check szip parameters for validity, plus all other outstanding szip work... #1629

merged 44 commits into from
Feb 12, 2020

Conversation

edwardhartnett
Copy link
Contributor

@edwardhartnett edwardhartnett commented Feb 7, 2020

There are some invalid parameters for szip that HDF5 will only report at enddef time. This is not very nice for users! ;-)

So now setting szip with invalid parameters will be detected by netcdf-c, and the user will get a NC_EINVAL, and all will be right with the world again.

This PR includes all other outstanding szip PRs, which are required to get to this point. ;-)

Fixes #1615
Fixes #1622
Fixes #1473
Fixes #1618
Fixes #1616
Fixes #1612

@edwardhartnett edwardhartnett changed the title check szip parameters for validity check szip parameters for validity, plus all other outstanding szip work... Feb 7, 2020
@edwardhartnett
Copy link
Contributor Author

@WardF this is ready to merge. It has passed all your CI and all my CI! ;-)

This will complete the szip work and close all szip related issues.

@edwardhartnett
Copy link
Contributor Author

@WardF can we get this PR merged soon? It contains a lot of important changes and I don't want the PR to go stale. ;-)

Thanks,Ed

@DennisHeimbigner
Copy link
Collaborator

I concur with Ed, but for a different reason. The multifilter code
I built is going to mess strongly with Ed's szip-related changes.
So I need it be in the master as a stable base against which
to modify the multifilter code to match his changes.

@WardF WardF merged commit 0a9e5c3 into Unidata:master Feb 12, 2020
@WardF
Copy link
Member

WardF commented Feb 12, 2020

Thanks, both of you!

@edwardhartnett
Copy link
Contributor Author

edwardhartnett commented Feb 12, 2020

Well @DennisHeimbigner that makes me nervous! ;-)

But I have changed the szip code to use nc_def_var_filter(), so hopefully that will make some of your changes easier.

Also @WardF thanks for all the help getting these changes merged. These new compression methods have already made a big difference for NOAA and all NOAA data users.

@DennisHeimbigner
Copy link
Collaborator

Adding support for multiple filters turned out to be a big change.
In order to avoid a lot of special casing, I chose to bring szip and deflate
into the more general framework.

@edwardhartnett edwardhartnett deleted the ejh_more_szip_7 branch March 6, 2020 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment