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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MNT]: _ImageBase.draw and Axis.draw args and kwargs #27745

Closed
rcomer opened this issue Feb 4, 2024 · 4 comments · Fixed by #27768
Closed

[MNT]: _ImageBase.draw and Axis.draw args and kwargs #27745

rcomer opened this issue Feb 4, 2024 · 4 comments · Fixed by #27768
Milestone

Comments

@rcomer
Copy link
Member

rcomer commented Feb 4, 2024

Summary

The draw methods of _ImageBase and Axis take *args and **kwargs but do not do anything with them. The Ribbon Box example has an artist that inherits from AxesImage and also passes *args and **kwargs through draw.

Proposed fix

Based on #16022 my guess is that *args and **kwargs should be removed from the signatures, though I don't know much about when and how draw gets called. If they should be removed, do we need a deprecation period?

@tacaswell
Copy link
Member

This is internal API (it is called by the Aritsts parent as part of rendering the figure (the update methods in the GUIs and the savefig methods eventually create a Renderer instance and then call Figure.draw(renderer) which is turn uses the visitor pattern to pass the renderer to all of the (nested) children and the result is the accumulated state on the renderer (for Agg that is an RGBA buffer we can extract, for svg it is a file-like we have been streaming text into).).

No one should be calling this outside of the draw process so I don't think we need an deprecation period, but we should highlight it in the release notes (and be prepared to add a deprecation period if we actually broke somenone).

@rcomer
Copy link
Member Author

rcomer commented Feb 6, 2024

Thanks for the explanation @tacaswell! I ended up here because I was looking at an artist in Cartopy whose draw method will update colors, etc. from kwargs if passed. I couldn’t think under what circumstances they would be passed so went grepping for other examples, but didn’t get any less confused!

@timhoffm
Copy link
Member

timhoffm commented Feb 6, 2024

If we want to be on the very safe side, we can still do a deprecation. There's no hurry in removing the arguments.

@tacaswell
Copy link
Member

tacaswell commented Feb 6, 2024

Once you are down the path of writing your own draw methods you could have a class that calls subsequent classes and does pass something meaningful across the function call, but if downstream libraries are doing that, that is between them and themselves (so long as they do not require the kwargs it matches our expectations and I see no reason we should care if they are optional).

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

Successfully merging a pull request may close this issue.

4 participants