-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Update find_nearest_contour
and revert contour deprecations
#27088
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
Conversation
lib/matplotlib/contour.py
Outdated
segment : int | ||
The index of the `.Path` in *contour* that is closest to | ||
The index within that closest path of the segment that is closest to |
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'm using "segment" here to mean one of the pieces you get when you split the path by its MOVETOs. I'm not sure I have the language right though. What exactly defines a segment?
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.
connected component, I think
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.
The word 'segment' is a bit confusing, because Paths have a method iter_segments
, which splits by code, i.e., a MOVETO returns one point, a CURVE3 returns two points, etc. This does not appear to be the same as the 'segment' used here.
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.
So should I change “segment” to “connected component” here? Or something else?
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 guess it could be a 'subpath'?
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.
"subpath" is what I have called the variable when I've looped through these, so that is intuitive to me 👍
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.
OK I have changed it to "subpath" and also added some explanation of what paths and subpaths correspond to in the plot.
Updated based on #27070 (comment). There is a possibility for all the return values to be |
7b84ca1
to
521c4d9
Compare
I have a slight preference for using the _cm_set approach for handling the pixel=False (if that indeed works, untested) so that the backcompat code doesn't start interfering with the "main intended usage" code. |
521c4d9
to
691549e
Compare
It does work. It just seemed odd to me to essentially hack the method's behaviour from outside when directly changing the behaviour was very simple. I will defer to you on that though. |
94cc465
to
61a434b
Compare
find_nearest_contour
to not use collections
attributefind_nearest_contour
and revert contour deprecations
Co-authored-by: Antony Lee <anntzer.lee@gmail.com>
Co-authored-by: Ruth Comer <10599679+rcomer@users.noreply.github.com>
61a434b
to
e9a102f
Compare
…t contour deprecations
…088-on-v3.8.x Backport PR #27088 on branch v3.8.x (Update `find_nearest_contour` and revert contour deprecations)
PR summary
Following [Bug]: find_nearest_contour deprecated with no replacement? #27070 (comment), this PR updates the
ContourSet.find_nearest_contour
method so it no longer uses the deprecatedcollections
attribute.Note that the docstring previously stated in error that the first element of the returned tuple is a
Collection
. It was actually an integer that indexed thecollections
attribute. Since <3.8 behaviour had one collection per level, and in 3.8 we have one path per level I have updated this to say the index of the path.contour.allsegs
,contour.allkinds
, andcontour.find_nearest_contour
are no longer marked for deprecation.Closes #27070 and closes #26913
PR checklist