Skip to content

Conversation

doronbehar
Copy link
Contributor

PR summary

Mostly addresses #30393. One may argue the API is still inconsistent, but at least it is documented now.

PR checklist

@doronbehar doronbehar force-pushed the _Buttons--label_props.doc branch from 95c4dda to 5493009 Compare August 12, 2025 23:38
Copy link
Member

@dstansby dstansby left a comment

Choose a reason for hiding this comment

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

Thanks - this looks good now. There's a couple of links that need fixing though, I think (but haven't tested) that my inline suggestions to reference the classes instead of __init__ should work.

@doronbehar doronbehar force-pushed the _Buttons--label_props.doc branch from 5493009 to f7376b1 Compare August 13, 2025 10:34
@dstansby dstansby added this to the v3.10.6 milestone Aug 13, 2025
@ksunden
Copy link
Member

ksunden commented Aug 14, 2025

Not sure if we want to roll it into here or do it separately, but looks like the type hint for label_props should also be updated.

label_props: dict[str, Any] | Sequence[dict[str, Any]] | None = ...,

That type hint was based on reading the docstring, but it is clear from the code that Sequence/list was not actually a viable input to this parameter. (Namely, there is a check_is_instance for dict/None)

That was the one change that flagged to me as "oh, this is actually changing the documented type, we should be sure of that" and upon looking, I agree with the new version, just looking to keep the type hints in sync if we can.

@doronbehar
Copy link
Contributor Author

Thanks for noticing this @ksunden. I think it is appropriate to fix this in this PR too - type hints are sort of a documentation. Pushed a fix.

@doronbehar doronbehar requested a review from dstansby August 14, 2025 21:55
@doronbehar doronbehar force-pushed the _Buttons--label_props.doc branch from d6073f1 to 0ef0da4 Compare August 14, 2025 22:00
@doronbehar doronbehar force-pushed the _Buttons--label_props.doc branch from 0ef0da4 to 2e10b79 Compare August 14, 2025 22:01
@dstansby dstansby merged commit 443ec06 into matplotlib:main Aug 20, 2025
38 of 40 checks passed
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Aug 20, 2025
timhoffm added a commit that referenced this pull request Aug 20, 2025
…412-on-v3.10.x

Backport PR #30412 on branch v3.10.x ({Check,Radio}Buttons: Improve docs of label_props)
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.

4 participants