Skip to content

numcodecs.zfpy is ready #229

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

Merged
merged 84 commits into from
Mar 18, 2021
Merged

numcodecs.zfpy is ready #229

merged 84 commits into from
Mar 18, 2021

Conversation

halehawk
Copy link
Contributor

@halehawk halehawk commented Apr 21, 2020

numcodecs.zfpy is ready with pip install from https://test.pypi.org/simple/ zfpy==0.5.5rc1, this package has no py38 build and osx build.

TODO:

  • Unit tests and/or doctests in docstrings
  • tox -e py38 passes locally
  • Docstrings and API docs for any new/modified user-facing classes and functions
  • Changes documented in docs/release.rst
  • tox -e docs passes locally
  • AppVeyor and Travis CI passes
  • Test coverage to 100% (Coveralls passes)

@lindstro
Copy link

CI appears to be failing on Python 3.9. Guess it is just due to lack of wheels on that platform. Suggested some changes above to fix that

I can add 3.9 wheels. Someone recently requested this, but at the time I wasn't sure if we require MB_ML_VER=2010 on Linux as we do for 3.8. I can't seem to find the thread where this was discussed. Does anyone know?

@jakirkham
Copy link
Member

Honestly I think a new release of zfpy is probably more important than Python 3.9 support. I don’t know how much work that would be though

@halehawk
Copy link
Contributor Author

halehawk commented Mar 18, 2021 via email

@lindstro
Copy link

Honestly I think a new release of zfpy is probably more important than Python 3.9 support. I don’t know how much work that would be though

We've essentially frozen new features and are working on tests and documentation for zfp 0.5.6. Shouldn't be too much longer. But once we do the release, it would make sense to include Python 3.9.

@jakirkham
Copy link
Member

Yep that makes sense. Knew that work towards a release was ongoing. So didn't want to ask for more things, but yeah agree that it makes sense to support Python 3.9 at the same time

@halehawk
Copy link
Contributor Author

halehawk commented Mar 18, 2021 via email

@jakirkham
Copy link
Member

jakirkham commented Mar 18, 2021

Could we just skip building the docs on Python 3.9? Maybe build them on Python 3.8 instead?

@halehawk
Copy link
Contributor Author

halehawk commented Mar 18, 2021 via email

@jakirkham jakirkham merged commit d16d1ea into zarr-developers:master Mar 18, 2021
@jakirkham
Copy link
Member

Thank you @halehawk! 😄

@pjpetersik
Copy link

pjpetersik commented Sep 30, 2021

@rabernat and @halehawk What is the reason that the ZFPY Codec actually uses ensure_contiguous_ndarray instead of ensure_ndarray?

When I was playing around with this Codec for compression of weather model data, i.e. 2m temperature from ICON-EU, replacing ensure_contiguous_ndarray with ensure_ndarray did not led to any errors and led to higher compression ratios.

Am I running in some problems if I replace ensure_contiguous_ndarray with ensure_ndarray in a custom ZFPY Codec?

@halehawk
Copy link
Contributor Author

halehawk commented Sep 30, 2021 via email

@pjpetersik
Copy link

@halehawk I meant ensure_contiguous_ndarray in the ZFPY Codec class:

buf = ensure_contiguous_ndarray(buf)

@jakirkham
Copy link
Member

Many of the compressors we use don't know how to handle shape correctly hence why it does that

We could add a flag to ensure_contiguous_ndarray to skip the reshape step. Then use that flag here

@halehawk
Copy link
Contributor Author

halehawk commented Sep 30, 2021 via email

@jakirkham
Copy link
Member

@halehawk it probably will be a while before I have time to do that. In the midst of doing several release atm. If you or @pjpetersik , would like to do a PR though would be happy to review :)

@halehawk
Copy link
Contributor Author

halehawk commented Sep 30, 2021 via email

@halehawk
Copy link
Contributor Author

halehawk commented Nov 2, 2021 via email

@jakirkham
Copy link
Member

Maybe it would be best to move this discussion to a new issue where it is easier to track

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants