Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Line2D._path obeys drawstyle.#6497
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Line2D._path obeys drawstyle.#6497
Uh oh!
There was an error while loading.Please reload this page.
Conversation
For stepping drawstyles, `Line2D.draw` used to recompute a path atdrawing time, because its `_path` attribute always assumed a lineardrawstyle. Instead, directly compute the correct path.Alsofixesmatplotlib#6447 (`Line2D.contains` did not take drawstyle into account)because that code relied on proximity of the mouse event with theunderlying path.
| drawStyles = {} | ||
| drawStyles.update(_drawStyles_l) | ||
| drawStyles.update(_drawStyles_s) | ||
| # Need a list ordered with long names first: | ||
| drawStyleKeys = (list(six.iterkeys(_drawStyles_l)) + | ||
| list(six.iterkeys(_drawStyles_s))) | ||
| _drawstyle_conv = { | ||
| 'default': lambda x, y: (x, y), | ||
| 'steps': pts_to_prestep, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Can you just add these two toSTEP_LOOKUP_MAP and correct the spelling ofstep-pre and friends tosteps-*? I suspect I meant to do the refactor you are doing now as part of the work where thefill_between feature got added, but ran out of steam (it was one of the last things to get merged before 1.5.0 iirc).
What the input to I would keep the |
Like in this version? |
| 'post': pts_to_poststep, | ||
| 'mid': pts_to_midstep, | ||
| 'step-pre': pts_to_prestep, | ||
| 'step-post': pts_to_poststep, | ||
| 'step-mid': pts_to_midstep} | ||
| 'step-mid': pts_to_midstep, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I would remove thestep-* entries. They are not documented and this is the only place those strings appear in the code base. I am 98% sure the lack of 's' here was my mistake.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Done.
db60992 tob8fe72cCompare👍 Will merge once CI finishes. |
matplotlib#6497 set the `_path` attribute of Line2D to the actually drawn patheven if the drawstyle is `steps-*`; however containment tests and thesubslice optimization for very long paths were not updated accordingly(seematplotlib#6615). This patch fixes the issues.Note that `contains` returns, for events in a horizontal segment of a"steps-mid" drawstyle plot, the index of the point in the segment,regardless of whether the event occured to the left or the right ofthat point.
For stepping drawstyles,
Line2D.drawused to recompute a path at drawing time, because its_pathattribute always assumed a linear drawstyle. Instead, directly compute the correct path.Alsofixes#6447 (
Line2D.containsdid not take drawstyle into account) because that code relied on proximity of the mouse event with the underlying path.Note that unfortunately, the drawstyle names remain inconsistent with those accepted by
fill_betweenandfill_betweenx, which (seem to be the only functions to) rely oncbook.STEP_LOOKUP_MAP. It may be a good opportunity to fix the discrepancy too...