Skip to content
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

doc: fix Patch.contains_point docstring example #27681

Merged
merged 1 commit into from Jan 23, 2024

Conversation

hassec
Copy link
Contributor

@hassec hassec commented Jan 22, 2024

PR summary

See the docstring of Patch.contains_point:

Notes
-----
The proper use of this method depends on the transform of the patch.
Isolated patches do not have a transform. In this case, the patch
creation coordinates and the point coordinates match. The following
example checks that the center of a circle is within the circle
>>> center = 0, 0
>>> c = Circle(center, radius=1)
>>> c.contains_point(center)
True
The convention of checking against the transformed patch stems from
the fact that this method is predominantly used to check if display
coordinates (e.g. from mouse events) are within the patch. If you want
to do the above check with data coordinates, you have to properly
transform them first:
>>> center = 0, 0
>>> c = Circle(center, radius=1)
>>> plt.gca().add_patch(c)
>>> transformed_center = c.get_transform().transform(center)
>>> c.contains_point(transformed_center)
True

It explains that one has to use data coordinates when using contains_point but then provides an example that uses Circle and get_transform which isn't correct. This has lead to confusion in the past, see #23178.

The proposed changes in this PR will move the example to use get_data_transform and uses values which would fail if one were to use get_transform.

IMHO this closes #23178.

PR checklist

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks.

You can also join us on gitter for real-time discussion.

For details on testing, writing docs, and our review process, please see the developer guide

We strive to be a welcoming and open project. Please follow our Code of Conduct.

@dstansby dstansby added Documentation Documentation: API files in lib/ and doc/api labels Jan 22, 2024
@timhoffm
Copy link
Member

To convince me and others that this change is correct, I used the following. Note that the only difference between the two plots is get_transform() vs get_data_transform() in the last two lines.

import numpy as np
import matplotlib.pyplot as plt
from matplotlib.patches import Circle

fig, (ax1, ax2) = plt.subplots(1, 2, figsize=(6, 3))

c1 = Circle((0, 0), radius=3)
c2 = Circle((0, 0), radius=3)
ax1.add_patch(c1)
ax2.add_patch(c2)
points = 8* (np.random.random((200, 2)) - 0.5)
ax1.scatter(points[:, 0], points[:, 1], c=[c1.contains_point(c1.get_transform().transform(p)) for p in points])
ax2.scatter(points[:, 0], points[:, 1], c=[c1.contains_point(c1.get_data_transform().transform(p)) for p in points])

image

@timhoffm timhoffm added this to the v3.8.3 milestone Jan 23, 2024
@timhoffm timhoffm merged commit 5cf9697 into matplotlib:main Jan 23, 2024
47 checks passed
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Jan 23, 2024
@timhoffm
Copy link
Member

Thanks @hassec for figuring this out. Congratulations on your first contribution to Matplotlib! We'd love to see you back.

QuLogic added a commit that referenced this pull request Jan 24, 2024
…681-on-v3.8.x

Backport PR #27681 on branch v3.8.x (doc: fix Patch.contains_point docstring example)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation: API files in lib/ and doc/api Documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: contains_point() does not appear to work?
3 participants