Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

MNT: deprecate draw method args and kwargs #27768

Merged
merged 2 commits into from
Feb 14, 2024

Conversation

rcomer
Copy link
Member

@rcomer rcomer commented Feb 10, 2024

PR summary

Closes #27745

I am very much appreciating how easy the deprecation module makes this 馃憤

PR checklist

@rcomer
Copy link
Member Author

rcomer commented Feb 11, 2024

I am confused by the stubtest failure, which I have reproduced locally

note: unused allowlist entry matplotlib.image._ImageBase.draw

_imageBase.draw is not in the allowlist. The only draw in there is Axis.draw, which this PR also modifies, but removing that line from the allowlist does not get rid of the error.

@dstansby
Copy link
Member

dstansby commented Feb 12, 2024

Had a bit of a dig, and that's because stubtest.py sneakily adds anything that is decorated
with "delete_parameter" to another allowlist (self.output is an allowlist file):

def visit_FunctionDef(self, node):
if any("delete_parameter" in ast.unparse(line) for line in node.decorator_list):
parents = []
if hasattr(node, "parent"):
parent = node.parent
while hasattr(parent, "parent") and not isinstance(parent, ast.Module):
parents.insert(0, parent.name)
parent = parent.parent
self.output.write(f"{'.'.join(self.context + parents)}.{node.name}\n")

I'm not sure why the purpose of this is, and why it's not playing ball with matplotlib.image._ImageBase.draw though...

@ksunden
Copy link
Member

ksunden commented Feb 12, 2024

Ahh... when used for non-*args/**kwargs, delete_parameter adds a sentinel default value which breaks typing, so rather than specifically enumerating and causing the list of exceptions to have to be updated (or the type hints to have to be updated leaking private internal sentinel values...) I just automatically detected when it was used.

However, for variadic args no such sentinel is used, so it doesn't actually break so the entry is unused.

@ksunden
Copy link
Member

ksunden commented Feb 12, 2024

Pushed an update to the automatic detection code to parse out when a vararg/kwarg is delete_parameter'd instead of just always adding it. Basically just unwinds the for loop from the any clause and adds some checks where it will continue before adding the exception (and a break to avoid adding the same exception twice, which I don't think would be a problem but might as well)

Also added a comment that at least indicates the reason it is there/what is happening.

@rcomer
Copy link
Member Author

rcomer commented Feb 12, 2024

Thanks @ksunden!

@QuLogic QuLogic added this to the v3.9.0 milestone Feb 13, 2024
@timhoffm timhoffm merged commit 05df015 into matplotlib:main Feb 14, 2024
41 of 42 checks passed
@rcomer rcomer deleted the draw-args-kwargs branch February 14, 2024 20:46
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.

[MNT]: _ImageBase.draw and Axis.draw args and kwargs
5 participants