Skip to content

WIP: Add plot method to digitization#6412

Closed
GuillaumeFavelier wants to merge 4 commits intomne-tools:masterfrom
GuillaumeFavelier:dig_plot
Closed

WIP: Add plot method to digitization#6412
GuillaumeFavelier wants to merge 4 commits intomne-tools:masterfrom
GuillaumeFavelier:dig_plot

Conversation

@GuillaumeFavelier
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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
Copy Markdown
Contributor Author

Is the docstring better now @agramfort ?

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

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

@massich
Copy link
Copy Markdown
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