Deprecators #4464

hmaarrfk · 2020-02-18T02:38:22Z

Description

@jni, @alexdesiqueira this is the demo that I was talking about regarding how the decorator can be used to basically make the change "once".

In version 1.0, you are left with "zombie" decorators that would basically do nothing.

https://github.com/hmaarrfk/wabisabi/blob/master/wabisabi/kwonly_change.py#L89
I forced push to my old PR, now i can't reopen it.
#3346

Checklist

Docstrings for all functions
Gallery example in ./doc/examples (new features only)
Benchmark in ./benchmarks, if your changes aren't covered by an
existing benchmark
Unit tests
Clean style in the spirit of PEP8

For reviewers

Check that the PR title is short, concise, and will make sense 1 year
later.
Check that new functions are imported in corresponding __init__.py.
Check that new features, API changes, and deprecations are mentioned in
doc/release/release_dev.rst.
Consider backporting the PR with @meeseeksdev backport to v0.14.x

pep8speaks · 2020-02-18T02:58:10Z

Hello @hmaarrfk! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

In the file skimage/transform/hough_transform.py:

Line 9:1: E302 expected 2 blank lines, found 1
Line 118:80: E501 line too long (80 > 79 characters)
Line 230:80: E501 line too long (81 > 79 characters)

In the file skimage/transform/radon_transform.py:

Line 381:80: E501 line too long (84 > 79 characters)

In the file skimage/transform/tests/test_radon_transform.py:

Line 319:11: E114 indentation is not a multiple of four (comment)
Line 319:11: E116 unexpected indentation (comment)

Comment last updated at 2020-03-02 00:33:34 UTC

soupault · 2020-02-18T09:28:51Z

This is a pretty cool thing for long-term maintenance 👍.

rfezzani · 2020-02-18T10:21:58Z

Very cool stuff. What about #4158 then? Close, postpone....

alexdesiqueira · 2020-02-26T01:49:25Z

A small complaining on the tests, @hmaarrfk:

/home/travis/venv/lib/python3.7/site-packages/skimage/transform/radon_transform.py:docstring of skimage.transform.iradon:57:Inline literal start-string without end-string.

Could you check it, please? :)

hmaarrfk · 2020-03-02T00:53:09Z

So if people are in favor of this, I suggest we either:

A. Version wabisabi (well what we need from it)
B. Depend on it

and start doing this kind of deprecation 1 file at a time, taking care of internal warnings at every step of the way.

This is a "one time necessary change" with the change acting as a no-op if the version specified has passed.

alexdesiqueira · 2020-03-02T02:06:08Z

I'm +1 here, but I have two questions to help others decide @hmaarrfk:

A. Version wabisabi (well what we need from it)

How difficult (and heavy) would that be?

B. Depend on it

How heavy would that be?

hmaarrfk · 2020-03-02T03:06:23Z

The only dependency is numpydoc for documentation mangling.

That said, it is optional, and for many people it will be installed automatically when they install Spyder.

Versioning it would require copying in 3 files or so, maybe more for tests, but the license is already included in the files, and I can give my blessing to provide full copyright to scikit image if it helps.

rfezzani · 2020-03-02T08:54:47Z

This is a "one time necessary change" with the change acting as a no-op if the version specified has passed.

Even if the decorators acts punctually, I don't think that it is a good idea to keep ghost/useless lines in the code base... Even if it doesn't hurt to keep these lines, it would be a good idea to plan some cleanings at release time...

hmaarrfk · 2020-03-02T13:21:09Z

Yes of course, but I don't think it can be considered "technical Dept" just an annoyance in the codebase that would require minimal review to be pushed through

rfezzani · 2020-03-03T09:05:45Z

skimage/version.py

@@ -0,0 +1,2 @@
+# Avoid circular imports


😢 We really need to dive in skimage import strategy to fix it...

skimage/_shared/_deprecators.py

rfezzani

Thank you @hmaarrfk really cool ;-)

Edit

~~What if we move deprecate_kwarg from skimage._shared.utils to wabisabi?~~
What if we replace skimage._shared.utils.deprecate_kwarg by wabisabi.kwarg_name_change? 😄

jni

@hmaarrfk I've suggested a very small change. What's the status with wabisabi? Are you ok making breaking changes there? Overall, I absolutely love the functionality and am now convinced to depend on it.

Having said this, this could not be used nicely for e.g. changes in rescale behaviour. (As far as I can see?) ie you'd need to introduce preserve_ranges everywhere and then remove them eventually, which is double the pain of just changing it at 1.0...

But, in general, I really like this functionality and can see myself using it both in 0.x and in the 1.0->2.0 transition.

jni · 2020-03-16T03:28:49Z

skimage/transform/hough_transform.py


-
-def hough_line_peaks(hspace, angles, dists, min_distance=9, min_angle=10,
+@kwonly_change('1.0')


Ironically, I think this should be a keyword-only argument. 😂

Suggested change

@kwonly_change('1.0')

@kwonly_change(until_version='1.0')

jni · 2020-03-16T03:29:30Z

skimage/transform/radon_transform.py

-def iradon(radon_image, theta=None, output_size=None,
+@kwonly_change('1.0', previous_arg_order=['radon_image', 'theta',
+                                          'output_size', 'filter_name',
+                                          'interpolation', 'circle'])


Whoa. This is a super-cool feature.

hmaarrfk · 2020-03-16T05:08:58Z

Rescale behavior is a little tougher. We can try to write a spec for itto see how the input output response would look like.

I would rather not make breaking changes there, but I don't think I use the kwonly deprecator in my production code so maybe it is ok to break?

You can suggest a PR there if you like

jni · 2020-03-16T09:49:38Z

I would rather not make breaking changes there

Aw. But it would be so delightfully meta to use wabisabi on wabisabi itself! 😂 Anyway, I'll have a look. My main proposals right now are:

kwonly_change -> update_keyword_only[_args]
version -> *, until_version

The function definition itself can be simplified by using toolz.curry and having func be the only positional argument.

hmaarrfk · 2020-03-16T14:58:02Z

do you really want to depend on toolz?

until_version makes sense

do you want all the functions to start with update_??

jni · 2020-03-17T06:14:37Z

do you really want to depend on toolz?

Yep, pretty lightweight, pure python, and extremely stable. Dask depends on it so it's not even like it's unmaintained.

do you want all the functions to start with update_??

Maybe? I like my code to read as close to English as possible. And it's equivalent to having them all end with _change. ;)

grlee77 · 2021-04-05T14:08:22Z

A deprecate_kwarg utility was merged in #4158 and that same file also has decorators for remove_arg and change_default_value .

Is there additional functionality here that we still want to consider or can this PR be closed?

hmaarrfk · 2021-04-05T14:54:53Z

No. I don't think so. I was following your progress and it seems you added the important features.

hmaarrfk added 7 commits February 22, 2020 10:39

Demo using deprecators

d805378

forgot a *,

5f9200c

build typos on my end.

f28b198

Fixup a few things

28182fc

Update pyramid test

e118218

Cleanup internal use of pyramid

c8be659

Show a few more examples

0421860

hmaarrfk force-pushed the deprecators branch from bc5bcc3 to 0421860 Compare February 22, 2020 15:45

hmaarrfk added 2 commits March 1, 2020 19:00

cleanup a few bugs

3cadcb5

Remove warnings for kwonly in test suite

fdf5d19

rfezzani reviewed Mar 3, 2020

View reviewed changes

skimage/version.py

@@ -0,0 +1,2 @@

# Avoid circular imports

Copy link

Member

rfezzani Mar 3, 2020

Choose a reason for hiding this comment

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

😢 We really need to dive in skimage import strategy to fix it...

rfezzani reviewed Mar 3, 2020

View reviewed changes

skimage/_shared/_deprecators.py Show resolved Hide resolved

rfezzani reviewed Mar 3, 2020

View reviewed changes

skimage/_shared/_deprecators.py Show resolved Hide resolved

rfezzani reviewed Mar 3, 2020

View reviewed changes

rfezzani added ⚖️ Needs decision 🔧 type: Maintenance Refactoring and maintenance of internals labels Mar 5, 2020

rfezzani approved these changes Mar 5, 2020

View reviewed changes

jni requested changes Mar 16, 2020

View reviewed changes

rfezzani mentioned this pull request Mar 30, 2020

Size of structuring elements a bit inconsistent #4536

Open

rfezzani mentioned this pull request Apr 21, 2020

WIP: Turn off relabelling in label2rgb #3015

Open

7 tasks

Base automatically changed from master to main February 18, 2021 18:23

mkcor mentioned this pull request Feb 22, 2021

2021's calendar of community management #5169

Closed

grlee77 added the 🤖 type: Infrastructure CI, packaging, tools and automation label Apr 5, 2021

hmaarrfk closed this Apr 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deprecators #4464

Deprecators #4464

hmaarrfk commented Feb 18, 2020

pep8speaks commented Feb 18, 2020 •

edited

soupault commented Feb 18, 2020

rfezzani commented Feb 18, 2020

alexdesiqueira commented Feb 26, 2020 •

edited

hmaarrfk commented Mar 2, 2020

alexdesiqueira commented Mar 2, 2020

hmaarrfk commented Mar 2, 2020

rfezzani commented Mar 2, 2020

hmaarrfk commented Mar 2, 2020

rfezzani Mar 3, 2020

rfezzani left a comment •

edited

jni left a comment

jni Mar 16, 2020

jni Mar 16, 2020

hmaarrfk commented Mar 16, 2020

jni commented Mar 16, 2020

hmaarrfk commented Mar 16, 2020

jni commented Mar 17, 2020

grlee77 commented Apr 5, 2021

hmaarrfk commented Apr 5, 2021



		def hough_line_peaks(hspace, angles, dists, min_distance=9, min_angle=10,
		@kwonly_change('1.0')

Deprecators #4464

Deprecators #4464

Conversation

hmaarrfk commented Feb 18, 2020

Description

Checklist

For reviewers

pep8speaks commented Feb 18, 2020 • edited

Comment last updated at 2020-03-02 00:33:34 UTC

soupault commented Feb 18, 2020

rfezzani commented Feb 18, 2020

alexdesiqueira commented Feb 26, 2020 • edited

hmaarrfk commented Mar 2, 2020

alexdesiqueira commented Mar 2, 2020

hmaarrfk commented Mar 2, 2020

rfezzani commented Mar 2, 2020

hmaarrfk commented Mar 2, 2020

rfezzani Mar 3, 2020

Choose a reason for hiding this comment

rfezzani left a comment • edited

Choose a reason for hiding this comment

Edit

jni left a comment

Choose a reason for hiding this comment

jni Mar 16, 2020

Choose a reason for hiding this comment

jni Mar 16, 2020

Choose a reason for hiding this comment

hmaarrfk commented Mar 16, 2020

jni commented Mar 16, 2020

hmaarrfk commented Mar 16, 2020

jni commented Mar 17, 2020

grlee77 commented Apr 5, 2021

hmaarrfk commented Apr 5, 2021

pep8speaks commented Feb 18, 2020 •

edited

alexdesiqueira commented Feb 26, 2020 •

edited

rfezzani left a comment •

edited