-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Remove deprecated functions and arguments for the 0.19 release #5463
Conversation
Implemented a couple of remaining deprecations missing from the TODO as found by Also fixed f2878e5 by adding the removal of |
(segmentation.watershed should be used instead)
c685e76
to
97743cd
Compare
* In ``skimage/segmentation/morphsnakes.py`` remove ``circle_level_set'' and related tests. | ||
* Also make sure to look in the function ``_init_level_set`` | ||
* In ``skimage/morphology/_flood_fill.py`` replace the deprecated parameter | ||
in flood_fill() ``inplace`` with ``in_place`` and update the tests. |
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 didn't follow when or why this change was decided on... pandas speaks 'inplace' so it seems more standard to me. Oh well.
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 don't remember, but I think it was for consistency with several other functions that use in_place
. It looks like a number of functions under future/graph
already have in_place
, also morphology.remove_small_holes
and morphology.remove_small_objects
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.
That said, most of those are still in future, so changes are probably expected there. I wonder if there are other prominent examples of inplace
aside from pandas?
I don't think SciPy or NumPy tend to use either, usually requiring an out
or output
argument to be specified instead.
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 don't remember, but I think it was for consistency with several other functions that use
in_place
. It looks like a number of functions underfuture/graph
already havein_place
Ok.
also
morphology.remove_small_holes
andmorphology.remove_small_objects
Right, but it looks like the argument is going away altogether for 1.0!
scikit-image/skimage/morphology/misc.py
Line 51 in 7b0d37d
@remove_arg("in_place", changed_version="1.0", |
And going NumPy-style instead:
scikit-image/skimage/morphology/misc.py
Line 52 in 7b0d37d
help_msg="Please use out argument instead.") |
Co-authored-by: Marianne Corvellec <marianne.corvellec@ens-lyon.org>
Thanks for reviewing this fairly long PR @mkcor! |
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.
Thank you @grlee77, just left 2 minor suggestions.
Co-authored-by: Riadh Fezzani <rfezzani@gmail.com>
I believe these are copies of test cases already in NumPy
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.
Excellent, thank you @grlee77. just waiting for the green CI before merge 😉
Description
This PR implements the remaining deprecations listed under the 0.19 section of
TODO.txt
. These are broken up into separate small commits for easier reviewing.Checklist
./doc/examples
(new features only)./benchmarks
, if your changes aren't covered by anexisting benchmark
For reviewers
later.
__init__.py
.doc/release/release_dev.rst
.