BUG: Prevent crash on repr of recursive array#8963
Conversation
Previously, formatters could incur errors from being run on object arrays, even though the formatter was not used.
be255d6 to
c3b84e7
Compare
numpy/core/arrayprint.py
Outdated
There was a problem hiding this comment.
What's the point of this? (forgive my ignorance)
There was a problem hiding this comment.
Not polluting the global namespace. Previously there was also from functools import reduce.
As for why that's there - reduce is no longer a global in python 3
ahaldane
left a comment
There was a problem hiding this comment.
The lambda part looks good.
The wrapper part seems fine too. It doesn't protect against recursion where 'self' changes each iteration, as in:
>>> np.set_printoptions(formatter={'all': lambda x: str(np.array([x]))})but I don't think we need that.
numpy/core/arrayprint.py
Outdated
There was a problem hiding this comment.
I think it would be good to add a comment here that this is to account for (for example) self-containing object arrays.
There was a problem hiding this comment.
Good point - I'll fix that up when I find the time (or else, you can if you don't want to wait - this PR is editable)
There was a problem hiding this comment.
All right, I will wait. Otherwise it looks good to merge to me.
c3b84e7 to
1e65a2a
Compare
|
|
||
|
|
||
| # gracefully handle recursive calls - this comes up when object arrays contain | ||
| # themselves |
There was a problem hiding this comment.
@ahaldane: Done - seemed more appropriate to comment "why" at the call-site
| argument, it returns `fillvalue` instead of recursing. | ||
|
|
||
| Largely copied from reprlib.recursive_repr | ||
| """ |
There was a problem hiding this comment.
Docstring improved too.
|
LGTM. I'm going to go ahead and merge. Thanks @eric-wieser ! |
|
Great! Let's see if this makes #8983 easier |
Also adds a release note for numpygh-8963
Also adds a release note for numpygh-8963
Fixes #8960, by:
array2string(within a single thread)np.maximumon arrays which might be recursive - there's no point setting up anIntegerFormatterif we're not yet sure we're working with integers