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

fix colorbar suppression when vlim is joint #11867

Merged
merged 2 commits into from
Aug 7, 2023

Conversation

drammock
Copy link
Member

@drammock drammock commented Aug 7, 2023

@larsoner
Copy link
Member

larsoner commented Aug 7, 2023

Looks like the default is colorbar=False which might explain why the tutorial has no colorbar

https://output.circle-artifacts.com/output/job/a2b760da-5862-4658-bf9e-0082d15d00c9/artifacts/0/html/auto_tutorials/epochs/20_visualize_epochs.html#sphx-glr-auto-tutorials-epochs-20-visualize-epochs-py

image

So it looks the same as on main

https://mne.tools/dev/auto_tutorials/epochs/20_visualize_epochs.html#sphx-glr-auto-tutorials-epochs-20-visualize-epochs-py

That being said I think they both look okay and the code looks okay, so if you agree this (still) does not need a colorbar then +1 for merge from me based on my having looked at the code (it should do the right thing I think)

FWIW at some point we could switch this to use constrained_layout and just add a fig.colorbar rather than using this ai == len(axes) - 1 or whatever. But that would be for another PR I think :)

@larsoner larsoner added this to the 1.5 milestone Aug 7, 2023
@drammock
Copy link
Member Author

drammock commented Aug 7, 2023

That's the wrong image to check. It's the last image before this section https://output.circle-artifacts.com/output/job/a2b760da-5862-4658-bf9e-0082d15d00c9/artifacts/0/html/auto_tutorials/epochs/20_visualize_epochs.html#plotting-epochs-as-an-image-map

And it now (correctly) has only 1 colorbar

@larsoner larsoner merged commit 7c4e27c into mne-tools:main Aug 7, 2023
21 checks passed
@larsoner
Copy link
Member

larsoner commented Aug 7, 2023

Yes indeed, thanks @drammock !

@drammock drammock deleted the fix-vlim-joint branch August 8, 2023 13:47
snwnde pushed a commit to snwnde/mne-python that referenced this pull request Mar 20, 2024
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