Skip to content

MNT: revert contour deprecations #27075

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

Closed

Conversation

jklymak
Copy link
Member

@jklymak jklymak commented Oct 12, 2023

contour.allsegs, contour.allkinds, and contour.find_nearest_contour are no longer marked for deprecation.

These are useful for post-processing and perhaps extra labelling. It pins us down a bit more to have them available, but the benefit justifies the cost in my opinion.

Closes #27070 #26913

@jklymak jklymak force-pushed the mnt-revert-contour-deprecations branch from 9ec3c40 to 60dc1f7 Compare October 12, 2023 22:01
@jklymak jklymak requested a review from anntzer October 12, 2023 22:03
@jklymak jklymak marked this pull request as ready for review October 12, 2023 22:03
@jklymak jklymak force-pushed the mnt-revert-contour-deprecations branch from 60dc1f7 to d68f01a Compare October 12, 2023 22:04
@jklymak jklymak force-pushed the mnt-revert-contour-deprecations branch from d68f01a to 536582f Compare October 12, 2023 22:05
@anntzer
Copy link
Contributor

anntzer commented Oct 12, 2023

For find_nearest_contour that won't really work, as you'd also need to undeprecate the collections property, which is accessed by its implementation (as detailed in #27070 (comment)), and then you'd need to decide whether accessing that property just permanently triggers the conversion to the old-style internal representation or not (currently it does as that's the point of the backcompat layer). If it does then that means that any ContourSet on which one calls find_nearest_contour will revert back to a state where #6139, #11464, #23080 are broken. Perhaps we should keep that discussion at #27070 though.

I can't say I like keeping allsegs and allkinds around forever, but I don't really mind either.

@jklymak
Copy link
Member Author

jklymak commented Oct 12, 2023

@anntzer apologies - I didn't see your note on #27070. Lets move this to draft and discuss that first...

@jklymak jklymak marked this pull request as draft October 12, 2023 22:59
@rcomer
Copy link
Member

rcomer commented Oct 19, 2023

I have pulled this change into #27088.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: find_nearest_contour deprecated with no replacement?
4 participants