Skip to content

Fix default return of Collection.get_{cap,join}style #25810

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 28, 2023
Merged

Fix default return of Collection.get_{cap,join}style #25810

merged 1 commit into from
Jul 28, 2023

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented May 4, 2023 •
edited
Loading

PR summary

If neither are specified at object creation, the default is to be None. This broke get_{cap,join}style when the enum wrappers were created as they assume the internal value is always an enum value.

$ python -c 'from matplotlib.collections import Collection; print(Collection().get_capstyle())'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File ".../lib/matplotlib/collections.py", line 643, in get_capstyle
    return self._capstyle.name
AttributeError: 'NoneType' object has no attribute 'name'
$ python -c 'from matplotlib.collections import Collection; print(Collection().get_joinstyle())'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File ".../lib/matplotlib/collections.py", line 657, in get_joinstyle
    return self._joinstyle.name
AttributeError: 'NoneType' object has no attribute 'name'

This was broken since the initial addition of the enums in #18544, so I'm not sure if it should be backported (minus the typing changes.)

PR checklist

@ksunden
Copy link
Member

ksunden commented May 4, 2023 •
edited
Loading

The alternative would be to resolve to the default. As far as I can tell the default is hard coded here, so not controlled by rcparam. I suppose backends could override the default? But not convinced that edge case is worth accounting for.

@QuLogic
Copy link
Member Author

QuLogic commented May 4, 2023

Yes, this was added in #9523 and, if None, defaulted to backend renderer state. Also, #17575 documented this as defaulting to the patch.{cap,join}style rcParam, but that doesn't appear to be true. Alternatively we could start defining a default for collections as documented.

Either way, we should probably fix the default and documented default to match.

@ksunden ksunden added this to the v3.8.0 milestone Jul 26, 2023
@ksunden
Copy link
Member

ksunden commented Jul 26, 2023

Test failure is almost certainly the MacOS/Azure/Tk problem we were having.

@QuLogic
Fix default return of Collection.get_{cap,join}style
4b99bc6 
If neither are specified at object creation, the default is to be
`None`. This broke `get_{cap,join}style` when the enum wrappers were
created as they assume the internal value is always an enum value.
@QuLogic QuLogic force-pushed the fix-default-collection-style branch from 2f4f08e to 4b99bc6 Compare July 27, 2023 07:50
@greglucas greglucas merged commit 3ca6f23 into matplotlib:main Jul 28, 2023
@QuLogic QuLogic deleted the fix-default-collection-style branch July 28, 2023 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ksunden ksunden ksunden approved these changes

@greglucas greglucas greglucas approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v3.8.0
Development

Successfully merging this pull request may close these issues.
3 participants
@QuLogic @ksunden @greglucas