MRG, ENH - Plot optodes in plot_alignment for fNIRS channels

[kylemath]

Reference issue

Example: Addressing discussion in other PR on loading BOXY data here: #7717.

What does this implement/fix?

In addition to plotting the midpoint of source and detector, the source and detector locations should also be plot-able, to evaluate coregistration. Plotting is done in plot_alignment based on other_picks.

Example of desired output currently working in first pass, black are detectors, red sources, midpoints not shown: (-Colours - what is reserved, what is good?, )

First try by creating two new keys in other_loc based on other points, resetting other_keys, and adding colours and scales to the DEFAULTS

    for key, picks in other_picks.items():
        if other_bools[key] and len(picks):
            if key == 'fnirs':
                # other_loc[key] = np.array([info['chs'][pick]['loc'][:3]
                #                for pick in picks])
                other_loc['source'] = np.array([info['chs'][pick]['loc'][3:6]
                                               for pick in picks])
                other_loc['detector'] = np.array([info['chs'][pick]['loc'][6:9]
                                               for pick in picks])
                other_keys = sorted(other_loc.keys())
                logger.info('Plotting %d %s location%s'
                            % (len(other_loc[key]), key, _pl(other_loc[key])))

Additional information

Potential API needed to add:
@rob-luke says:
Currently plot alignment takes the arguments fnirs=('pairs', 'channels'). You could open another PR to extend this to fnirs=('pairs', 'channels', 'sources', 'detectors')?

@rob-luke says:
Currently channel plotting uses other_picks, you would need to extend the code to plot ['loc'][3:6], and ['loc'][6:9] based on if sources or detectors was passed somewhere around here https://github.com/mne-tools/mne-python/blob/master/mne/viz/_3d.py#L1039

Other ideas outside scope

• perhaps think about plotting banana shaped path between source and detector instead of shortest path (easiest way is the 3d shape made by a string of spheres: https://github.com/kylemath/MOpt3d/blob/master/setuplookup.m)

• perhaps think about plotting path along scalp curve instead of shortest linear path

[kylemath]

@rob-luke third times a charm, here is a nice clean PR finally as a start for plotting the optodes

[rob-luke]

The attached figure looks great.

black are detectors, red sources, midpoints not shown: (-Colours - what is reserved, what is good?, )

Like I mentioned in the other PR, I think red is an intuitive choice for an infrared emitter. Others would need to comment if black is already reserved.

perhaps think about plotting banana shaped path

I think this would be more appropriate for MNE-NIRS. Do these "banana" shapes usually represent the most probable photon migration path? If so, would we need to implement mne-tools/mne-nirs#62 first?

perhaps think about plotting path along scalp curve instead of shortest linear path

Yes, improving the pairs option to plot along the scalp would be a nice improvement.

[larsoner]

Others would need to comment if black is already reserved.

Can you look at plot_alignment and see if black is used anywhere? Maybe just use a dark gray (0.85 or so?), I doubt we've used that for anything but it should still hopefully look okay against the gray surfaces.

[kylemath]

@rob-luke - just thinking of api now, you suggest: fnirs=('pairs', 'channels', 'sources', 'detectors')

Is the idea with plotting the "channels" midpoint that this is where the data is assignment in the latter head plots in your analysis pipeline?

[kylemath]

maybe something like this:

    if fnirs is True:
        fnirs = ['pairs', 'sources', 'detectors']

    for xi, x in enumerate(fnirs):
        _check_option('fnirs[%d]' % xi, x, ('channels', 'pairs', 'sources', 'detectors'))

    for key, picks in other_picks.items():
        if other_bools[key] and len(picks):
            if key == 'fnirs':
                if 'channels' in fnirs:
                    other_loc[key] = np.array([info['chs'][pick]['loc'][:3]
                                   for pick in picks])
                if 'sources' in fnirs:
                    other_loc['source'] = np.array([info['chs'][pick]['loc'][3:6]
                                                   for pick in picks])
                if 'detectors' in fnirs:
                    other_loc['detector'] = np.array([info['chs'][pick]['loc'][6:9]
                                               for pick in picks])
                other_keys = sorted(other_loc.keys())
                logger.info('Plotting %d %s location%s'
                            % (len(other_loc[key]), key, _pl(other_loc[key])))

[kylemath]

@rob-luke - this is ready for an initial review but I think everything wanted is in the current PR

[rob-luke]

I would prefer to leave the default plotting option as it is. I think having the additional sources and detectors creates too much visual clutter (particularly when you extend this to multimodal datasets). Further, I think that having pairs and sources and detectors is redundant. So please leave the default as is, and if the users want the additional points they can pass in sources and detectors to the fnirs argument.

You will need to add some tests too. Previously this was done by checking that the correct items were listed in the logs. And there are still seems to be some debugging print statements in the PR.

[kylemath]

thanks @rob-luke good feedback, agree on the default changes,
can you point me to where the tests for this function would live

[rob-luke]

Tests tend to live in a test directory in the same location as the file you are editing. You can also click on the details of the failing tests below it will show you the tests that are failing and the path to them (here is a link for current failing CI). Its also handy to look at the commit history of the file you are editing, the previous changes are usually accompanied by changes to tests and documentation that you can mimic.

[kylemath]

@rob-luke - fixed the default behaviour and reverted the colour of the channels to the red it was before, fixed bug, and added tests of logs

[kylemath]

saw the tests, got my flake8 plugin working, and fixing these style problems now

[rob-luke]

saw the tests, got my flake8 plugin working, and fixing these style problems now

Great. It took me a for attempts to understand all the different tests, but after you've been through it once its easier. Having flake8 and pydocstyle installed locally saved me quite some time too.

Following the process here... "When you believe a PR is ready to be reviewed and merged, convert it from a draft PR to a normal PR, change its title to begin with MRG, and add a comment to the PR asking for reviews (changing the title does not automatically notify maintainers)." Then people will review your code.

[kylemath]

@rob-luke @larsoner I believe this is ready to go, could I please have a review?

[rob-luke]

Thanks for sharing the figures. After looking at them I definitely don't like the white channels and white pairs, its too hard to see the channels. Currently my preference is for your original colouring (sorry to send you in circles), but somehow accentuating the difference between the red and orange.

Perhaps get the tests passing, then when others review they can weigh in on the colours.

[kylemath]

ok thanks, looks like this for now while I work on tests

[kylemath]

And with that last change the tests will all pass now @larsoner @rob-luke

[larsoner]

Otherwise LGTM

[kylemath]

@larsoner @rob-luke addressed all those comments in last commit

[kylemath]

Sorry about that @larsoner, Should be unchanged from master now,
thanks for guiding me through slowly,

[larsoner]

LGTM but let's wait to get the approval from @rob-luke to merge

[kylemath]

not sure what happened with the travis test:

FAILED mne/inverse_sparse/tests/test_gamma_map.py::test_gamma_map - assert 3....
Results (1805.46s):
    1794 passed
       2 xpassed
       1 failed
         - mne/inverse_sparse/tests/test_gamma_map.py:46 test_gamma_map
       3 xfailed
      29 skipped

[larsoner]

Fixed by #7764, I restarted the runs

[rob-luke]

The updated tutorial figure looks great. Thanks @kylemath.

[larsoner]

Thanks @kylemath !

[kylemath]

awesome thanks for the help learning,