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

WIP: Add plot method to digitization #6412

Closed

Conversation

GuillaumeFavelier
Copy link
Contributor

Reference issue

Fixes #6411.

What does this implement/fix?

Reuse the plot_alignment() function in the plot() method of the Digitization class.

@GuillaumeFavelier
Copy link
Contributor Author

For so small modifications, I'm surprised to have such a strange error:
ImportError: cannot import name '_dig_kind_dict' from 'mne.digitization.base'

meg=None, eeg='original',
dig=False, ecog=True, src=None, mri_fiducials=False,
bem=None, seeg=True, show_axes=False, fig=None,
interaction='trackball', verbose=None):
Copy link
Member

Choose a reason for hiding this comment

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

@GuillaumeFavelier if you require all the same things as plot_alignment then I don't see the point.

dig.plot() should just work.
or if you wnat to couple with a subject:
dig.plot(trans, subject, subjects_dir)

@codecov
Copy link

codecov bot commented Jun 5, 2019

Codecov Report

Merging #6412 into master will decrease coverage by <.01%.
The diff coverage is 33.33%.

@@            Coverage Diff             @@
##           master    #6412      +/-   ##
==========================================
- Coverage   89.27%   89.26%   -0.01%     
==========================================
  Files         411      411              
  Lines       74477    74480       +3     
  Branches    12312    12312              
==========================================
- Hits        66487    66485       -2     
- Misses       5133     5137       +4     
- Partials     2857     2858       +1


Parameters
----------
trans : str | 'auto' | dict | None
Copy link
Member

Choose a reason for hiding this comment

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

docstring does not cover the "auto" and "dict" usage. Also this docstring should be made
a template in doc.py to be shared among functions that need a trans param

@GuillaumeFavelier
Copy link
Contributor Author

Is the docstring better now @agramfort ?

@@ -103,3 +103,29 @@ def __eq__(self, other): # noqa: D105
return False
else:
return all([ss == oo for ss, oo in zip(self, other)])

def plot(self, trans=None, subject=None, subjects_dir=None):
"""Plot head, sensor, and source space alignment in 3D.
Copy link
Member

Choose a reason for hiding this comment

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

this is no source space here.

please also add a tiny test

is None, an identity matrix is assumed.
subject : str | None
The subject name corresponding to FreeSurfer environment
variable SUBJECT. Can be omitted if ``src`` is provided.
Copy link
Member

Choose a reason for hiding this comment

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

src is not an option here.

@GuillaumeFavelier GuillaumeFavelier mentioned this pull request Jun 17, 2019
19 tasks
@GuillaumeFavelier GuillaumeFavelier self-assigned this Jun 17, 2019
@agramfort
Copy link
Member

this can be closed as DigMontage plot should rather be improved.

@massich
Copy link
Contributor

massich commented Sep 11, 2019

cross-referencing: #6649, #6744

@GuillaumeFavelier GuillaumeFavelier deleted the dig_plot branch January 8, 2020 15:12
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.

VIZ: Add a plot method to the digitization class
3 participants