Return ndarrays from encode and (where possible) decode
#136
Conversation
This file contains hidden or 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
Standardize the output of the `encode` function a bit by ensuring it returns an `ndarray`. This is done without copying as it only takes a view onto the data. Has the advantage of leveraging objects under the hood that might not appear as nice like objects' internal buffers without impacting the user or unnecessarily copying the data to `bytes`. Also makes it a bit easier to work with the output as users can do anything they would normally want to with `ndarray`s.
Where possible, ensure that `decode` returns an `ndarray`. Many codecs already do this except for some unusual ones that work with Python objects or text data. In the cases where the data is known to be binary, try to return `ndarray`s. This makes it pretty easy for end users of this data to do whatever they would like with it.
|
Please let me know how you want this listed in the changelog (if at all). |
alimanfoo
approved these changes
Nov 29, 2018
There was a problem hiding this comment.
Thanks @jakirkham, LGTM. Re changelog, I think it would be good to add something, how about something like:
* Return values from encode() and decode() methods are now returned as numpy arrays for consistency across codecs. By :user:`John Kirkham <jakirkham>`, :issue:`136`.
|
Went ahead and added release notes and resolved merge conflict, hope that's OK. Will merge if CI passes. |
|
Great, thanks for doing that, @alimanfoo. SGTM |
|
Apologies for not opening a new issue for this. Happy to do so if needed. Should we cut the release now or were there some other things still needed? |
|
No problem at all. I think cut the release now: #137. |
This was referenced Nov 30, 2018
8 tasks
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.
Ensures binary
encodefunctions returnndarrays generally. This makes the output easier to anticipate and work with. Also allows more unusual outputs like objects' internal buffers to be exposed in a friendly way to end users (while avoiding copying). Where possible also ensuredecodefunctions returnndarrays.TODO:
tox -e py37passes locallytox -e py27passes locallytox -e docspasses locally