Skip to content

Conversation

@larsoner
Copy link
Member

Should fix the bug mentioned in #7725. @christian-oreilly can you try it with your out-of-standard-ori MRI? I don't have any to test with but in principle I think this should work...

This PR also:

  1. Should ensure that we always (by default) proceed through slices in L-to-R, P-to-A, and I-to-S order (previously it depended on the order in of the MRI).
  2. Adds orientation markers to the edges of the plots by default (can be disabled with show_orientations=False)
  3. Points out that there is a redundant and probably also wrong (for non-conform'ed MRIs) function mne.viz._3d._plot_mri_contours that we should really refactor with the mne.viz.misc._plot_mri_contours -- the function names match and there is a lot of similar code so it's unfortunate that we ended up with two copies, presumably due to @agramfort 6 years ago accidentally making two copies in 5c1d64b :)
  4. Changes the default slices=None from:
    np.linspace(0, n_slices, 12, endpoint=False)
    
    to:
    np.linspace(0, n_slices - 1, 14, endpoint=True)
    
    There is almost never any reason to show the first or last slice. On master we correctly avoid the last but not the first and unevenly sample in between, we now evenly sample from near-to-first to near-to-last instead by making 14 points evenly spaced between first and last slice (inclusive) and then excluding the first and last of those 14 to obtain 12.

On master for plot_forward.py we get:
Screenshot from 2020-05-11 14-10-37

And on this PR:

Screenshot from 2020-05-11 14-20-32

Note that there is a left-to-right flip here. On master the subject's left was shown on the right (radiological convention, probably not intentional) and this PR sets it to the more standard (for us at least) neurological convention of having the subject's left on the left. You can tell this by the fact that in the image from this PR, slice 177 (bottom left) has the pill on the right, and then the last slice is flipped left-right relative to the master plots.

@larsoner
Copy link
Member Author

Reported as fixing orientations for @christian-oreilly here, so this one should be good to go

@agramfort
Copy link
Member

thanks for cleaning my mess @larsoner :)

@larsoner
Copy link
Member Author

Decided to take care of the deduplication here, it was't so bad actually. On master the BEM looks like:

Screenshot from 2020-05-11 17-07-24

And on this PR it looks like plot_bem:

Screenshot from 2020-05-11 17-05-25

@larsoner
Copy link
Member Author

Going to push one more fix to adjust the axial view, the left should really be on the left and the right on the right, not left on the bottom right on the top:

Screenshot from 2020-05-11 17-10-44

@larsoner
Copy link
Member Author

Style failure is just the one from #7767

@agramfort agramfort merged commit cd84e02 into mne-tools:master May 12, 2020
@agramfort
Copy link
Member

thx @larsoner

@larsoner larsoner deleted the bem branch May 12, 2020 11:41
larsoner added a commit to larsoner/mne-python that referenced this pull request May 19, 2020
* upstream/master: (74 commits)
  FIX: Correct a bug in find_bads_eog (mne-tools#7797)
  [MRG] split_naming='bids' changes from _part-%d to _split-%d (mne-tools#7794)
  MRG, MAINT, DOC: Remove spyder (mne-tools#7796)
  MAINT: fixes for linkcheck (mne-tools#7762)
  [WIP] Update ieeg data example for ECoG (mne-tools#7768)
  fix examples/tutorials [circle full] (mne-tools#7786)
  MAINT: Clean up VTK and add to pre on Azure (mne-tools#7780)
  ENH: Add matplotlib animation support [skip travis] (mne-tools#7783)
  MRG, API: change out_type default in permutation_cluster_(1samp_)test (mne-tools#7781)
  DOC: docstring fixes (mne-tools#7777)
  MRG, ENH: Add tol_kind option (mne-tools#7736)
  MRG, DOC: Notes about info (mne-tools#7772)
  ENH: Speed up NIRx read without preload (mne-tools#7759)
  Minor plot_raw aes improvement (mne-tools#7770)
  MRG, FIX: Fixes for BEM contours (mne-tools#7763)
  MRG, STY: Fix E741 (mne-tools#7767)
  MRG, ENH - Plot optodes in plot_alignment for fNIRS channels (mne-tools#7747)
  FIX: Update NIH support [skip travis] (mne-tools#7766)
  MAINT: Bump tol for gamma map test (mne-tools#7764)
  MRG, FIX: Fix MRI orientations (mne-tools#7725)
  ...
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