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

ENH: add tobytes and stop using tostring in documentation #4257

Merged
merged 1 commit into from
Feb 15, 2014

Conversation

juliantaylor
Copy link
Contributor

tostring returns bytes which are not equal to string, so provide a
tobytes function alias and deprecate tostring.

PyArray_ToString is not yet deprected in this PR, but should probably also be in favor of a more descriptive name.

I'm not sure if we should really deprecate tostring, it would force rdepends to add checks for both names if they want to support older versions without warnings.
We might also want an equivalent frombytes that does not accept strings for symmetry, but currently fromstring seems works for bytes and strings as it can also read formated strings instead of only raw bytes.

@juliantaylor
Copy link
Contributor Author

this also restores compatibility with older PIL versions of fromarray which expect a tobytes function in python3 due to a wrong source conversion.

@rgommers
Copy link
Member

rgommers commented Feb 4, 2014

Deprecation is not a good idea, for the reason you already mentioned.tostring is a widely used method and works perfectly fine on 2.x and also OK on 3.x except that there the name is misleading.

@rgommers
Copy link
Member

rgommers commented Feb 4, 2014

Which PIL versions by the way? Only Pillow and maybe the very last PIL version should be relevant these days.

@juliantaylor
Copy link
Contributor Author

removed the deprecation warning.

any PIL older than 2.1 should be affected, I hit it with 1.1.7 in ubuntu 13.10.

@rgommers
Copy link
Member

rgommers commented Feb 5, 2014

1.1.7 should be the lowest version that people still use, so will help some users.

@@ -5382,10 +5383,20 @@ def tolist(self, fill_value=None):
#........................
def tostring(self, fill_value=None, order='C'):
"""
This function is deprecated. Use tobytes() if support for older numpy
Copy link
Member

Choose a reason for hiding this comment

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

Can we call it something else than deprecated? That still sounds like something that will be removed at some point, which is not what should happen. Something like "This function has inconsistent behavior on Python 3.x; it's recommended to use tobytes instead if numpy 1.9 is the lowest version your code has to support".

@juliantaylor
Copy link
Contributor Author

removed the use of the word deprecated

@@ -9384,7 +9384,7 @@ self
\emph default
.
\series bold
tostring
tobytes
Copy link
Member

Choose a reason for hiding this comment

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

Down below there is "Return the bytes of this array in a Python string." that should be updated also.

@charris
Copy link
Member

charris commented Feb 9, 2014

Oops, looks like I commented in your repository version...

tostring returns bytes which are not equal to string, so provide a
tobytes function alias.
tostring does not emit a deprecation warning yet so rdepends do not need
to check two names to support older versions of numpy without warnings.
@charris
Copy link
Member

charris commented Feb 15, 2014

LGTM, thanks.

charris added a commit that referenced this pull request Feb 15, 2014
ENH: add tobytes and stop using tostring in documentation
@charris charris merged commit 3333728 into numpy:master Feb 15, 2014
@juliantaylor juliantaylor deleted the tostring-depr branch November 8, 2014 03:44
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.

None yet

3 participants