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 code-block #89

Merged
merged 9 commits into from
Mar 17, 2022
Merged

Fix code-block #89

merged 9 commits into from
Mar 17, 2022

Conversation

alexrockhill
Copy link
Contributor

Typo I missed

@alexrockhill
Copy link
Contributor Author

So there's also some overlap of the colorbars but it should look perfect after mne-tools/mne-python#10443

@alexrockhill
Copy link
Contributor Author

Ok done messing with stuff, this is really all the fixes

@alexrockhill
Copy link
Contributor Author

So the colorbar is all messed up because mne-tools/mne-python#10443 needs to stop the colorbar from being removed from gridspec. That can be added back after the 1.0 release. Right now on main, the title overlaps so it's really not great either way, will be totally fixed soon.

@adam2392
Copy link
Member

Looks like circleCI is failing.

@alexrockhill
Copy link
Contributor Author

alexrockhill commented Mar 17, 2022

Looks like circleCI is failing.

Sorry! It looks like it was an extra space # .. code-block not # .. code-block, dang.

Okay, once mne-tools/mne-python#10443 merges, there will be errors if colorbar_scale and colorbar_pos are None so I just wrapped that in a try-catch and changed them to None defaults. That is nice for maintainability to have those as None so we don't have to change them in both mne.viz.circle._plot_connectivity_circle and in mne-connectivity if we want to change their values, and I'm not sure the rationale for overriding the matplotlib defaults. We could keep the small colorbar as a default but it's much better not to have gridspec=False for the colorbar so that it doesn't clash with the plots when you try and subplots_adjust or tight_layout.

@alexrockhill
Copy link
Contributor Author

Ok, I think I was overthinking it, this just fixes those tiny leftovers and hopefully I did the code-block right this time

@agramfort agramfort merged commit f819d0f into mne-tools:main Mar 17, 2022
@alexrockhill alexrockhill deleted the circle branch March 17, 2022 20:46
@alexrockhill
Copy link
Contributor Author

tsbinns pushed a commit to tsbinns/mne-connectivity that referenced this pull request Dec 15, 2023
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.

None yet

3 participants