raise informative error message on misshapen input to set_offsets#22408
raise informative error message on misshapen input to set_offsets#22408ianhi wants to merge 2 commits intomatplotlib:mainfrom
set_offsets#22408Conversation
| modified = mcollections.Collection( | ||
| offsets=np.column_stack([[0, 1], [0, 1]]) | ||
| ) | ||
| modified.set_offsets([[], []]) |
There was a problem hiding this comment.
I think that this should actually be column_stack and I should get rid of the transpose in the set method. Most usage will be with column_stack i suspect.
There was a problem hiding this comment.
Please check what other set_offsets methods do. We should aim for consistency.
| (np.asarray(self.convert_xunits(offsets[:, 0]), 'float'), | ||
| np.asarray(self.convert_yunits(offsets[:, 1]), 'float'))) | ||
|
|
||
| if offsets.shape == (2, 0): |
There was a problem hiding this comment.
| if offsets.shape == (2, 0): | |
| if offsets.shape == (0, 2): |
Which will require some changes to the tests.
The docs say(N, 2) so this should be the degenerate case of ((N, 2), .., (2, 2), (1, 2), (0, 2)) (the (2,) case is a special case for spelling (1, 2)).
There was a problem hiding this comment.
I think (0,2) just works fine in the else block? This branch is necessary to handle the special case of offsets=[[],[]]
import numpy as np
np.asanyarray([[],[]]).shape
# (2, 0)There was a problem hiding this comment.
ah wait I think I understand better. You're saying that [[],[]] is not really a valid input? becuase that's like saying I have two points each with zero data, which doesn't make any sense I guess.
|
Anyone can dismiss my review |
|
Fell free to move out of draft... |
|
Looking at this again, I think my comment is still valid. |
set_offsets
|
Looking again myself I now understand and agree with @tacaswell 's comment. and have updated this PR to instead throw the error message that would have set me on the right track. |
PR Summary
Fixes: #22401
Importantly allows this:
PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand runflake8 --docstring-convention=all).Documentation
doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).