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.

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

DOC: Improve Image Slices Viewer example #24366

Merged
merged 1 commit into from Nov 6, 2022
Merged

DOC: Improve Image Slices Viewer example #24366

merged 1 commit into from Nov 6, 2022

Conversation

timhoffm
Copy link
Member

@timhoffm timhoffm commented Nov 4, 2022

PR Summary

closes #24360

@timhoffm timhoffm added this to the v3.6.3 milestone Nov 4, 2022
@timhoffm timhoffm mentioned this pull request Nov 4, 2022
Closed
jklymak
jklymak approved these changes Nov 4, 2022
View reviewed changes
Copy link
Member

@jklymak jklymak left a comment

Choose a reason for hiding this comment

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

We have that handle persistence note somewhere else as well. Do we need to find other places for it?

@timhoffm
Copy link
Member Author

timhoffm commented Nov 4, 2022

We have that handle persistence note somewhere else as well. Do we need to find other places for it?

Do you mean add a note other places where we need to keep objects alive?

One coud do that. There's at least the following examples with similar patterns in the Event handling examples:

plus probably additional ones in Widgets.

However, this is beyond what I'm willing to do right now. We can open an issue for it.

On a general note, I'm not very convinced of this pattern to create a complex object and bind it's methods to a figure. IMHO it would make more sense for the ImageTracker to create and own the figure.

@tacaswell tacaswell merged commit 1685cb4 into matplotlib:main Nov 6, 2022
@meeseeksmachine meeseeksmachine mentioned this pull request Nov 6, 2022
Merged
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Nov 6, 2022
@tacaswell @meeseeksmachine
Backport PR matplotlib#24366: DOC: Improve Image Slices Viewer example
dc0c750
@tacaswell
Copy link
Member

On a general note, I'm not very convinced of this pattern to create a complex object and bind it's methods to a figure. IMHO it would make more sense for the ImageTracker to create and own the figure.

The pattern of an independent object that knows about the Figure and provides some interaction does match how I have frequently used this sort of thing. I've either hand a couple of these that I turn on and off or have been embedding in a larger GUI where if each one tried to make its own Figure object it would have caused some trouble for me.

I'm also wary of going to pure-functions because in my experience I have wanted to pull state off of these classes.

@timhoffm timhoffm deleted the doc-image-slice-viewer branch November 6, 2022 09:36
timhoffm added a commit that referenced this pull request Nov 6, 2022
@timhoffm
Merge pull request #24381 from meeseeksmachine/auto-backport-of-pr-24…
3702fd5 
…366-on-v3.6.x

Backport PR #24366 on branch v3.6.x (DOC: Improve Image Slices Viewer example)
@ksunden ksunden mentioned this pull request Feb 20, 2023
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tacaswell tacaswell tacaswell approved these changes

@jklymak jklymak jklymak approved these changes

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

Successfully merging this pull request may close these issues.

[ENH]: imshow support for multiple slice image volume
3 participants
@timhoffm @tacaswell @jklymak