BUG: Fix unicode(unicode_array_0d) on python 2.7 #9201
Conversation
f17b8d3 to
af8e1e5
Compare
af8e1e5 to
0a980d9
Compare
numpy/core/src/multiarray/strfuncs.c
Outdated
| @@ -198,3 +206,35 @@ array_str(PyArrayObject *self) | |||
| } | |||
| return s; | |||
| } | |||
|
|
|||
| #ifdef NPY_PY3K | |||
numpy/core/src/multiarray/strfuncs.h
Outdated
| @@ -10,4 +10,9 @@ array_repr(PyArrayObject *self); | |||
| NPY_NO_EXPORT PyObject * | |||
| array_str(PyArrayObject *self); | |||
|
|
|||
| #ifdef NPY_PY3K | |||
There was a problem hiding this comment.
Missed that in the rebase, I guess
numpy/ma/core.py
Outdated
| @@ -3885,15 +3885,15 @@ def compress(self, condition, axis=None, out=None): | |||
| _new._mask = _mask.compress(condition, axis=axis) | |||
| return _new | |||
|
|
|||
| def __str__(self): | |||
| def __str__(self, str_type=str): | |||
There was a problem hiding this comment.
Maybe help our future selves by adding an explicit note that str_type can be removed once we drop support of python 2
There was a problem hiding this comment.
Though I'm also slightly wary of using different signatures for dunder methods. Before you know it, subclasses rely on it... Might it be an idea to make a new function _prepare_str(self) (better name surely possible...) which returns res without str applied and then use that in both __str__ and __unicode__? Or possibly just the masked print option enabled part as _fill_with_mask_for_str?
0a980d9 to
538bfea
Compare
|
This recurses horribly, because some of the scalar |
|
How about splitting off the implementation of |
|
I haven't read the code yet, but won't the recursion problems be fixed after #9139? |
|
Nope, because #9139 still recurses in |
The sole purpose of implementing The problem is invoking |
|
(that was to trick the first two commits into disappearing) |
538bfea to
f166959
Compare
f166959 to
4f49897
Compare
|
I've stolen a commit from #9332, and rebased this on top of it. |
numpy/ma/core.py
Outdated
| String representation. | ||
|
|
||
| """ | ||
| def __insert_masked_print(self): |
|
I'm a little on the fence whether Also, while I did remove the On the other hand, removing the style argument and special-casing the str of 0d arrays is nice in two ways:
But are these two benefits enough to justify the compatibility break of disabling the style argument? |
|
In my future perfect world, scalars would not exist at all, one would just have array scalars, and those behave like other arrays, so I'd prefer not to go in a direction where array scalars are less like arrays, even if it has the benefit of making them more similar to scalars. I'd could see going the other way, though, removing |
|
My argument for why I'll fix the warning this evening. |
|
@eric-wieser - I don't think that you should compare [1] https://docs.python.org/3/reference/datamodel.html#object.__str__ |
4450637 to
a8c563a
Compare
|
Another argument for not letting |
a8c563a to
988b158
Compare
988b158 to
940b4a1
Compare
|
Rebased on top of #9913, which makes the diff a little smaller |
940b4a1 to
72025eb
Compare
…ing_function It's more important that scalars and 0d arrays are consistent here. Previously, unicode(arr0d) would crash on 2.7
72025eb to
df0fff4
Compare
|
LGTM. I understand it is a little strange that after this PR, in python2 0d I'd like to merge. Good to go? (checking since you pushed without commenting) |
|
Yep, good to go.
I agree - this is better than the old behaviour, and sending it's also consistent with |
|
All right, merging. Thanks Eric! |
Related to #9139, first two commits stand alone in #9202Now
a commit from #9332a single commit on top of #9784, standalone.This is like #9332, but:
np.set_string_function(is this a good thing?)unicodesuch that they convert to python unicode scalarsResults on python 2.7: