Repository navigation
Expire deprecations in widgets and keyword only arguments for Selectors - #24254
greglucas merged 4 commits into
Conversation
Sorry, something went wrong.
|
I think this is only missing a removal note? |
Sorry, something went wrong.
It's decorated with delete_parameter, so it should be removed. |
Sorry, something went wrong.
|
Agree with Elliott that we can fully remove drawtype, but the UI won't let me add the patches. Also agree with Greg that this needs a removal note. |
Sorry, something went wrong.
|
The main thing here is the modified keyword only arguments. I wanted some feedback on that before continuing with release notes. But I take it that it makes sense, at least sort of. Will get to the release notes then. |
Sorry, something went wrong.
|
|
||
| def __init__(self, ax, xy, callback=None, useblit=True): | ||
| @_api.make_keyword_only("3.7", name="useblit") | ||
| def __init__(self, ax, xy, callback, useblit=True): |
There was a problem hiding this comment.
callback cannot be None, so it doesn't make sense to have a default value (despite this going back 17 years...).
Sorry, something went wrong.
There was a problem hiding this comment.
I'm inclinded that we want callback keyword-only as well.
FYI you can have keyword-only arguments without a default. This is valid:
def __init__(self, ax, xy, *, callback, useblit=True):
and would be called as
Lasso(ax, xy, callback=foo)
However, to be checked whether make_keyword_only does work correctly for such a situation.
Sorry, something went wrong.
There was a problem hiding this comment.
I'm a bit skeptical of changing callback to be keyword only. The only place we use this in a demo it is passed positionally. I am worried that we are going to get minor payoff at the cost of thrashing a bunch of user code.
Sorry, something went wrong.
There was a problem hiding this comment.
I would not make it kwonly; it should be pretty clear in the context that that argument is a callback. The point of making e.g. useblit kwonly is that if you write Lasso(ax, xy, somefunc, True), it is really not clear what True is (useblit=True is clearer), but it is quite obvious that somefunc is a callback (especially as it'll typically have a more meaningful name, e.g. onselect; if it's a lambda defined inline (Lasso(ax, xy, lambda vertices: ...)) the situation is also clear).
Sorry, something went wrong.
| def __init__(self, ax, onselect=None, useblit=True, props=None, | ||
| button=None): | ||
| @_api.make_keyword_only("3.7", name="useblit") | ||
| def __init__(self, ax, onselect, useblit=True, props=None, button=None): |
There was a problem hiding this comment.
onselect cannot be None (and isn't in any other Selector).
Sorry, something went wrong.
|
|
||
| def __init__(self, ax, xy, callback=None, useblit=True): | ||
| @_api.make_keyword_only("3.7", name="useblit") | ||
| def __init__(self, ax, xy, callback, useblit=True): |
There was a problem hiding this comment.
I'm inclinded that we want callback keyword-only as well.
FYI you can have keyword-only arguments without a default. This is valid:
def __init__(self, ax, xy, *, callback, useblit=True):
and would be called as
Lasso(ax, xy, callback=foo)
However, to be checked whether make_keyword_only does work correctly for such a situation.
Sorry, something went wrong.
|
|
||
| - The *lineprops* argument is removed, use *props* instead. | ||
| - The ``onpress`` and ``onrelease`` methods are removed. They are straight | ||
| aliases for ``press`` and ``release``. |
There was a problem hiding this comment.
Removal of TextBox.DIST_FROM_LEFT is missing.
Sorry, something went wrong.
There was a problem hiding this comment.
FWIW this is all just copied and pasted from the 3.5 release notes and deprecated replaced with removed (more or less).
But I will of course edit accordingly.
Sorry, something went wrong.
There was a problem hiding this comment.
Looks like this was deprecated in bd9ee5e but we did not get a API change note with it.
Sorry, something went wrong.
There was a problem hiding this comment.
Should I update this to be deprecated in 3.7?
Sorry, something went wrong.
There was a problem hiding this comment.
It has been warning, but was not documented. The conservative thing to do is put it back, document it removed in 3.7 and drop it in 3.8, but I'm inclined to be a bit YOLO here and just document that it is removed in 3.7 as this is a very niche feature...
Sorry, something went wrong.
|
I think the outstanding tasks here are:
|
Sorry, something went wrong.
|
Optional: #24254 (comment) It would be nice to directly include callback while we're at making things kwonly. But if that's to complicated, we can defer that. |
Sorry, something went wrong.
Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
We do not actually use it in the code so also remove setting it.
…ord only arguments for Selectors
…254-on-v3.7.x Backport PR #24254 on branch v3.7.x (Expire deprecations in widgets and keyword only arguments for Selectors)
…rs (matplotlib#24254) * Expire deprecations in widgets and keyword only arguments for Selectors
…rs (matplotlib#24254) * Expire deprecations in widgets and keyword only arguments for Selectors
…rs (matplotlib#24254) * Expire deprecations in widgets and keyword only arguments for Selectors
…rs (matplotlib#24254) * Expire deprecations in widgets and keyword only arguments for Selectors
…rs (matplotlib#24254) * Expire deprecations in widgets and keyword only arguments for Selectors
…rs (matplotlib#24254) * Expire deprecations in widgets and keyword only arguments for Selectors
…rs (matplotlib#24254) * Expire deprecations in widgets and keyword only arguments for Selectors
…rs (matplotlib#24254) * Expire deprecations in widgets and keyword only arguments for Selectors
…rs (matplotlib#24254) * Expire deprecations in widgets and keyword only arguments for Selectors
…rs (matplotlib#24254) * Expire deprecations in widgets and keyword only arguments for Selectors
…rs (matplotlib#24254) * Expire deprecations in widgets and keyword only arguments for Selectors
PR Summary
Will add notes etc, but think about the keyword-only aspect of Selectors/Widgets. Also, lineprops is unused in RectangleSelector, but I guess I cannot really remove it?
PR Checklist
Tests and Styling
Documentation