Skip to content

Use buffer_tobytes on non-object types in test #122

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 2 commits into from
Nov 9, 2018
Merged

Use buffer_tobytes on non-object types in test #122

merged 2 commits into from
Nov 9, 2018

Conversation

jakirkham
Copy link
Member

Pulled out from PR ( #121 )

When running the check_backwards_compatibility test, make sure not to call buffer_tobytes on object arrays from fixture data. Casting these to bytes doesn't really make sense as it will return representation of pointers in the underlying buffer. Not only are the pointers things we do not want to be comparing, but it is also internal spec to NumPy and Python that really isn't reliable either. So make sure to pass through the object arrays as is.

[Description of PR]

TODO:

  • Unit tests and/or doctests in docstrings
  • tox -e py37 passes locally
  • tox -e py27 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)

When running the `check_backwards_compatibility` test, make sure not to
call `buffer_tobytes` on `object` arrays from fixture data. Casting
these to bytes doesn't really make sense as it will return
representation of pointers in the underlying buffer. Not only are the
pointers things we do not want to be comparing, but it is also internal
spec to NumPy and Python that really isn't reliable either. So make sure
to pass through the `object` arrays as is.
@alimanfoo
Copy link
Member

Thanks for catching this!

Copy link
Member

@alimanfoo alimanfoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, feel free to merge.

@jakirkham
Copy link
Member Author

Thanks for the review. :)

@jakirkham jakirkham merged commit b03531c into zarr-developers:master Nov 9, 2018
@jakirkham jakirkham deleted the fix_check_backwards_compatibility branch November 9, 2018 23:14
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.

2 participants