X Tutup
Skip to content

DOC: Fix some small issues#26657

Merged
ksunden merged 4 commits intomatplotlib:mainfrom
QuLogic:fix-doc-bugs
Aug 31, 2023
Merged

DOC: Fix some small issues#26657
ksunden merged 4 commits intomatplotlib:mainfrom
QuLogic:fix-doc-bugs

Conversation

@QuLogic
Copy link
Copy Markdown
Member

@QuLogic QuLogic commented Aug 31, 2023

PR summary

PR checklist

@QuLogic QuLogic added this to the v3.8.0 milestone Aug 31, 2023
Comment on lines +31 to +34
@lines.Line2D.axes.setter
def axes(self, new_axes):
self.text.axes = new_axes
lines.Line2D.axes.fset(self, new_axes)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is an unusual enough pattern, that it probably warrants a comment explaining it. (Just a simple # Override the axes property setter and potentially # Call the superclass property setter)

It works, and is probably about the best you can do to override the property setter/call the super-class version of the property setter.

Some things I thought about which would have made some sense to me, but don't seem to work in this context:

  • using @MyLine.axes.setter (MyLine is not available at class instantiation time)
    • I worried a bit about the setter accidentally overriding the parent classes setter, but that does not happen)
  • using super().axes.fset(new_axes) (super().axes resoves to the return of the getter, not the property descriptor)

@ksunden
Copy link
Copy Markdown
Member

ksunden commented Aug 31, 2023

Other than adding a comment explaining the unusual property setter override, LGTM

@ksunden ksunden merged commit 14a0c5b into matplotlib:main Aug 31, 2023
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Aug 31, 2023
@QuLogic QuLogic deleted the fix-doc-bugs branch August 31, 2023 19:03
ksunden added a commit that referenced this pull request Aug 31, 2023
…657-on-v3.8.x

Backport PR #26657 on branch v3.8.x (DOC: Fix some small issues)
@ksunden ksunden mentioned this pull request Sep 15, 2023
5 tasks
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.

3 participants

X Tutup