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

ENH: coreg legend #10459

Merged
merged 5 commits into from
Mar 28, 2022
Merged

Conversation

GuillaumeFavelier
Copy link
Contributor

This PR adds a legend to the coreg app. It helps make the association fiducial <> color:

image

image

It's an item of #8833

@GuillaumeFavelier GuillaumeFavelier self-assigned this Mar 25, 2022
@GuillaumeFavelier
Copy link
Contributor Author

I pushed a commit to adjust the size so that it is visible with the notebook 3d backend too. Now it's 10% of the scene:

notebook pyvistaqt
image image

@GuillaumeFavelier
Copy link
Contributor Author

This is ready for reviews from my end @agramfort, @larsoner

@@ -746,6 +747,11 @@ def _configure_picking(self):
self._on_pick
)

def _configure_legend(self):
mri_fids_legend_actor = self._renderer.legend(
labels=[("LPA", "red"), ("Nasion", "green"), ("RPA", "blue")])
Copy link
Member

Choose a reason for hiding this comment

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

can you avoid redefining here the colors of fiducials and define each color in only one place? besides can you use (0, 1, 0) for nasion in legend so it's more readable.

@GuillaumeFavelier
Copy link
Contributor Author

@agramfort I reused the values in DEFAULTS['coreg']:

image

@GuillaumeFavelier
Copy link
Contributor Author

I pushed a commit to fix:

E   ValueError: 
E   	Invalid color input: ([255.   0.   0.])
E   	Must be string, rgb list, or hex color string.  For example:
E   		color='white'
E   		color='w'
E   		color=[1, 1, 1]
E   		color='#FFFFFF'

@GuillaumeFavelier GuillaumeFavelier merged commit 933e35c into mne-tools:main Mar 28, 2022
@GuillaumeFavelier GuillaumeFavelier deleted the enh/coreg_legend branch March 28, 2022 15:55
@GuillaumeFavelier
Copy link
Contributor Author

Thank you @agramfort !

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