Skip to content

radius modification in contains_point function when linewidth is specified #19807

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.

Sign up for GitHub

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

Jump to bottom
Closed
Feyn-Man opened this issue Mar 28, 2021 · 2 comments · Fixed by #27462
Closed

radius modification in contains_point function when linewidth is specified #19807

Feyn-Man opened this issue Mar 28, 2021 · 2 comments · Fixed by #27462
Milestone
v3.9.0

Comments

@Feyn-Man
Copy link

Bug report

Bug summary
When specifyng linewidth in a patch object and then using the contains_point function the radius of this is set to linewidth and returns inconsistent results. This is somewhat documented here but I think this is an unwanted behaviour since linewidth is specified in points and not in data coordinates, while the contains_point function gonna search for those data points enclosed between the border of the path and half the linewidth.

Code for reproduction

import matplotlib.patches as mpatches
import matplotlib.pyplot as plt

circle_ec = mpatches.Circle((0,0), radius=1, ec='r', lw=1)
circle = mpatches.Circle((0,0), radius=1)
point = (0,1.5)
print(circle_ec.contains_point(point))        
print(circle.contains_point(point))

Actual outcome

True
False

Expected outcome

The expected output is False in both cases since the point (0, 1.5) is neither in the patch (Circle in this case) neither on the line of the edge. I propose to multiply _radius = self.get_linewidth() in the _process_radius function in the source code by a corrective factor.

Matplotlib version

  • Operating system: Windows 10
  • Matplotlib version: 3.3.4
  • Matplotlib backend: TkAgg
  • Python version:
  • Jupyter version (if applicable): 3.9.0
  • Other libraries:

Installation from pip
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 6, 2023

This issue has been marked "inactive" because it has been 365 days since the last comment. If this issue is still present in recent Matplotlib releases, or the feature request is still wanted, please leave a comment and this label will be removed. If there are no updates in another 30 days, this issue will be automatically closed, but you are free to re-open or create a new issue if needed. We value issue reports, and this procedure is meant to help us resurface and prioritize issues that have not been addressed yet, not make them disappear. Thanks for your help!

@github-actions github-actions bot added the status: inactive Marked by the “Stale” Github Action label Dec 6, 2023
@tacaswell tacaswell added this to the v3.9.0 milestone Dec 7, 2023
tacaswell added a commit to tacaswell/matplotlib that referenced this issue Dec 7, 2023
@tacaswell
DOC: clarify the default value of *radius* in Patch.contains_point
6f9707b 
... and friends.

closes matplotlib#19807
tacaswell added a commit to tacaswell/matplotlib that referenced this issue Dec 7, 2023
@tacaswell
DOC: clarify the default value of *radius* in Patch.contains_point
ae5e0c6 
... and friends.

closes matplotlib#19807
@tacaswell tacaswell mentioned this issue Dec 7, 2023
Merged
@tacaswell
Copy link
Member

https://matplotlib.org/stable/api/_as_gen/matplotlib.patches.Patch.html#matplotlib.patches.Patch.contains_point The input to contains_point is understood to be in the coordinate system of patch.get_transform() which is (for the vast majority of rendered Patches) screen space.

I opened a PR to document the logic to pick the default radius, but I think there is still a latent bug with screen space being in pixels, but the linewidth being in points so we are (in all cases with reasonable dpi) not extending the range far enough.

I am not sure we know enough about the source / target of the transforms to be able to reliably fix that problem.

@github-actions github-actions bot removed the status: inactive Marked by the “Stale” Github Action label Dec 8, 2023
tacaswell added a commit to tacaswell/matplotlib that referenced this issue Dec 20, 2023
@tacaswell
DOC: clarify the default value of *radius* in Patch.contains_point
ae79828 
... and friends.

closes matplotlib#19807
tacaswell added a commit to tacaswell/matplotlib that referenced this issue Dec 21, 2023
@tacaswell @oscargus @QuLogic
DOC: clarify the default value of *radius* in Patch.contains_point
c19ef0d 
... and friends.

closes matplotlib#19807

Co-authored-by: Oscar Gustafsson <oscar.gustafsson@gmail.com>
Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
timhoffm added a commit that referenced this issue Feb 17, 2024
@tacaswell @oscargus
@QuLogic @timhoffm
DOC: clarify the default value of *radius* in Patch.contains_point (#…
2fa85ee 
…27462)

... and friends.

closes #19807

Co-authored-by: Oscar Gustafsson <oscar.gustafsson@gmail.com>
Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
Impaler343 pushed a commit to Impaler343/matplotlib that referenced this issue Mar 8, 2024
@tacaswell @oscargus
@QuLogic @timhoffm @Impaler343
DOC: clarify the default value of *radius* in Patch.contains_point (m…
cf069a3 
…atplotlib#27462)

... and friends.

closes matplotlib#19807

Co-authored-by: Oscar Gustafsson <oscar.gustafsson@gmail.com>
Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
Impaler343 pushed a commit to Impaler343/matplotlib that referenced this issue Mar 14, 2024
@tacaswell @oscargus
@QuLogic @timhoffm @Impaler343
DOC: clarify the default value of *radius* in Patch.contains_point (m…
d82509f 
…atplotlib#27462)

... and friends.

closes matplotlib#19807

Co-authored-by: Oscar Gustafsson <oscar.gustafsson@gmail.com>
Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v3.9.0
Development

Successfully merging a pull request may close this issue.

DOC: clarify the default value of *radius* in Patch.contains_point tacaswell/matplotlib
2 participants
@tacaswell @Feyn-Man