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

Drop temporary workaround for get_codec #361

Merged
merged 2 commits into from
Dec 15, 2018
Merged

Drop temporary workaround for get_codec #361

merged 2 commits into from
Dec 15, 2018

Conversation

jakirkham
Copy link
Member

@jakirkham jakirkham commented Dec 10, 2018

Follow-up to PR ( #352 ) and PR ( #268 )

This workaround was added because get_codec was modifying its argument. However as of Numcodecs 0.6.0, get_codec does not modify its argument as it takes a copy and modifies the copy instead. ( zarr-developers/numcodecs#79 ) Given as we now require Numcodecs 0.6.0, this workaround is no longer needed. Hence we drop it.

TODO:

  • Add unit tests and/or doctests in docstrings
  • Add docstrings and API docs for any new/modified user-facing classes and functions
  • New/modified features documented in docs/tutorial.rst
  • Changes documented in docs/release.rst
  • Docs build locally (e.g., run tox -e docs)
  • AppVeyor and Travis CI passes
  • Test coverage is 100% (Coveralls passes)

This workaround was added because `get_codec` was modifying its
argument. However as of Numcodecs 0.6.0, `get_codec` does not modify its
argument as it takes a copy and modifies the copy instead. Given as we
now require Numcodecs 0.6.0, this workaround is no longer needed. Hence
we drop it.
@jakirkham
Copy link
Member Author

Should be good to merge I think. Please let me know if you have any concerns, @alimanfoo.

@jakirkham jakirkham merged commit 2c1a6e0 into zarr-developers:master Dec 15, 2018
@jakirkham jakirkham deleted the drop_get_codec_copy_workaround branch December 15, 2018 23:32
@jakirkham
Copy link
Member Author

Going ahead and merging this as it doesn't seem controversial (given it is only removing a workaround added after the last release that has already been addressed upstream in Numcodecs). Though if there are any concerns or issues, please let me know and I'd be happy to address them.

@jakirkham jakirkham added this to the v2.3 milestone Feb 18, 2019
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.

1 participant