Codecov Report

Merging #7725 into master will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #7725      +/-   ##
==========================================
- Coverage   90.24%   90.23%   -0.01%     
==========================================
  Files         455      455              
  Lines       84626    84698      +72     
  Branches    13401    13421      +20     
==========================================
+ Hits        76367    76429      +62     
- Misses       5392     5402      +10     
  Partials     2867     2867              

friendly ping to @christian-oreilly

thx @larsoner

@larsoner Sorry for the late feedback. I do not have the MRI that prompted me to submit the issue #7221 anymore because I went around the issue by preprocessing all my MRI to a "conform" space in the meantime. But I always end up having to move data between pipelines and almost always run into issues with incompatible coordinate systems. So I found some MRI from a different project (originally processed in CIVET rather than FreeSurfer) that were not preprocessed to a conform to space and that is what I got:

Before to PR:

After the PR:

So... the fix works to the extent that it puts the MRI in coronal view when it is supposed to put it in this view. I think the rest is more about what level of user-friendliness you want to support (i.e., whether or not you would like to have it 100% aware of coordinate system so that it would invert axes if needed to display it in a standard orientation or leave it to the user to provide their MRI in a given coordinate system). At least, right now, it does the volume along the requested axis.

FYI, there appears to be some bug in the management of paths:

In [6]: import mne                                                              

In [7]: import os                                                               

In [8]: mne.viz.plot_bem("ShNe_V1_anlm0.5r", os.environ["SUBJECTS_DIR"])        
Using surface: /usr/local/freesurfer/subjects/ShNe_V1_anlm0.5r/bem/inner_skull.surf
Using surface: /usr/local/freesurfer/subjects/ShNe_V1_anlm0.5r/bem/outer_skull.surf
Using surface: /usr/local/freesurfer/subjects/ShNe_V1_anlm0.5r/bem/outer_skin.surf
Out[8]: <Figure size 780x706 with 12 Axes>

In [9]: mne.viz.plot_bem("ShNe_V1_anlm0.5r")                                    
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-9-1980c44bfbfe> in <module>
----> 1 mne.viz.plot_bem("ShNe_V1_anlm0.5r")

~/code/mne-python/mne/viz/misc.py in plot_bem(subject, subjects_dir, orientation, slices, brain_surfaces, src, show, show_indices, mri)
    460 
    461     # Get the BEM surface filenames
--> 462     bem_path = op.join(subjects_dir, subject, 'bem')
    463 
    464     if not op.isdir(bem_path):

~/anaconda3/lib/python3.7/posixpath.py in join(a, *p)
     78     will be discarded.  An empty last part will result in a path that
     79     ends with a separator."""
---> 80     a = os.fspath(a)
     81     sep = _get_sep(a)
     82     path = a

TypeError: expected str, bytes or os.PathLike object, not NoneType

The call at "In [9]" should works as, according to the docstring, the subjects_dir argument can be left to None, in which case "the path is obtained by using the environment variable SUBJECTS_DIR." This error seems to have been introduced with this fix because it works fine when I revert to the MNE version I had before applying this fix.

Indeed I can see how this error was created. I can fix the flipping problem more easily if you can share their T1.mgz and bem directory with me privately...? In the meantime I can work on a fix either way

@larsoner Sure, I'll sent these to you.

@christian-oreilly can you first try #7763 ? I think I only need the files if that PR does not work.

@larsoner Sure, I'll do.

@larsoner Yep, works fine! Both the orientation and the path thing. Nice work! :)