Clean up RectangleSelector move code #21921
Conversation
lib/matplotlib/widgets.py
Outdated
| resize = self._active_handle and not move | ||
|
|
||
| if resize: | ||
| # If resizing, use xy/xy_press in a de-rotated frame |
There was a problem hiding this comment.
| # If resizing, use xy/xy_press in a de-rotated frame | |
| # use xy/xy_press in a de-rotated frame |
Optional: The code above aleady says it.
lib/matplotlib/widgets.py
Outdated
| - Re-size | ||
| - Contine the creation of a new shape | ||
| """ | ||
| xy = np.array([event.xdata, event.ydata]) |
There was a problem hiding this comment.
| xy = np.array([event.xdata, event.ydata]) | |
| xy = np.array([event.xdata, event.ydata]) |
Side note: I'm wondering if a small internal class _Point would make sense for such cases, basically namedtuple-like with arithmetics. So that we can do.
p = _Point(x, y)
2*p
p + (1, 2)
p.x
Arithmetics should still be faster than the numpy overhead.
There was a problem hiding this comment.
I am +-0 on this change, because even if it does make slightly more compact, I am not convinced that it makes the code more readable. To me this is a matter of taste and it doesn't really matter: I don't see it as an improvement or a regression!
Typically, for a matplotlib developer event.xdata or event.x has a meaning, while in case of xy or xy_press, this is a redirection and one need to look up what it is.
An alternative would be to add some convenience properties (xy, xydata) to the Event class. These properties could also support basic arithmetic, as suggested by @timhoffm in its comments.
ba685b5 to
6b3ea86
Compare
|
A good point - I've dropped the commit that changes the variable names - hopefully what remains is definitely an improvement? |
PR Summary
I am working towards improving
RectangleSelectorto allow rotation past +/- 45 degrees. This PR cleans up some of the movement code to:xyandxy_pressvariables to make the code more readablePR 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).