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

Simplify logic for adding a single axes #88

Merged
merged 1 commit into from
May 5, 2023

Conversation

dstansby
Copy link
Member

@dstansby dstansby commented May 5, 2023

  • Add a figure() property to NapariMPLWidget, to replace the documented .figure attribute that actually never existed.
  • Add a add_single_axes() helper method to NapariMPLWidget, that automatically adds a single axes and styles it.
  • Clarify that NapariMPLWidget is not responsible for adding Axes to the figure.

@dstansby dstansby requested review from samcunliffe and ruaridhg May 5, 2023 11:36
@dstansby dstansby marked this pull request as ready for review May 5, 2023 11:36
Copy link
Collaborator

@samcunliffe samcunliffe left a comment

Choose a reason for hiding this comment

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

Yep cool. It's now explicit that we're delegating responsibility for the axis to each widget (which wasn't super clear to me on first pass).

From a styling PoV, are you thinking we'll tweak apply_napari_colorscheme to be overrideable (like from a user-supplied stylesheet)?

# changing color of axes background to transparent
self.canvas.figure.patch.set_facecolor("none")
Copy link
Collaborator

Choose a reason for hiding this comment

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

The figure's facecolor doesn't need to be set?

No, it doesn't but I don't get why.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's already set in NapariMPLWidget.__init__()

Copy link
Collaborator

Choose a reason for hiding this comment

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

🤦 yep. Ty.

@samcunliffe samcunliffe added this pull request to the merge queue May 5, 2023
Merged via the queue into matplotlib:main with commit 2f5fde3 May 5, 2023
@dstansby dstansby deleted the adding-axes branch May 5, 2023 13:04
@dstansby
Copy link
Member Author

dstansby commented May 5, 2023

From a styling PoV, are you thinking we'll tweak apply_napari_colorscheme to be overrideable (like from a user-supplied stylesheet)?

I'm expecting it will eventually become mostly redundant and replaced by a style sheet.

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

Successfully merging this pull request may close these issues.

2 participants