Skip to content

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 29, 2016

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented May 29, 2016

For stepping drawstyles, Line2D.draw used to recompute a path at drawing time, because its _path attribute always assumed a linear drawstyle. Instead, directly compute the correct path.

Also fixes #6447 (Line2D.contains did 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_between and fill_betweenx, which (seem to be the only functions to) rely on cbook.STEP_LOOKUP_MAP. It may be a good opportunity to fix the discrepancy too...

For stepping drawstyles, `Line2D.draw` used to recompute a path at
drawing time, because its `_path` attribute always assumed a linear
drawstyle.  Instead, directly compute the correct path.

Also fixes matplotlib#6447 (`Line2D.contains` did not take drawstyle into account)
because that code relied on proximity of the mouse event with the
underlying path.
@tacaswell tacaswell added this to the 2.1 (next point release) milestone May 29, 2016
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,
Copy link
Member

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 to STEP_LOOKUP_MAP and correct the spelling of step-pre and friends to steps-*? I suspect I meant to do the refactor you are doing now as part of the work where the fill_between feature got added, but ran out of steam (it was one of the last things to get merged before 1.5.0 iirc).

@tacaswell
Copy link
Member

What the input to fill_between and fill_betweenx is matching is the input to ax.step (see #4433).

I would keep the draw_style values as locked down as possible, but let where (in step) and step (in fill_between take the version both with and without steps).

@anntzer
Copy link
Contributor Author

anntzer commented May 29, 2016

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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would remove the step-* 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@anntzer anntzer force-pushed the fix-line2d-contains-drawstyle branch from db60992 to b8fe72c Compare May 29, 2016 21:56
@tacaswell
Copy link
Member

👍 Will merge once CI finishes.

@tacaswell tacaswell merged commit a260058 into matplotlib:master May 29, 2016
@anntzer anntzer deleted the fix-line2d-contains-drawstyle branch May 29, 2016 23:30
anntzer added a commit to anntzer/matplotlib that referenced this pull request Jun 26, 2016
 matplotlib#6497 set the `_path` attribute of Line2D to the actually drawn path
even if the drawstyle is `steps-*`; however containment tests and the
subslice optimization for very long paths were not updated accordingly
(see matplotlib#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 of
that point.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Line2D.contains does not take drawstyle into account.
3 participants