FIX: Adding use_scalebars as a parameter.

[adam2392]

Adds "use_scalebars" as a boolean parameter into plot() and plot_raw().

Reference issue

Example: Fixes #7068

What does this implement/fix?

Added a boolean parameter to allow programmatic on/off of the unit scale bars.

[adam2392]

Example of it being done.

plot_rawdata_scalebars.pdf

[drammock]

In addition to my comments, your PR is failing our automated style checks. You can run them locally from the root MNE-Python directory by running "make flake", "make codespell", "make pydocstyle" etc. I think there might even be a "make style" directive that runs them all (reviewing from my phone so relying on memory here)

[GuillaumeFavelier]

I think there might even be a "make style" directive that runs them all

make pep should do the trick.

More details in mne-python Makefile.

[adam2392]

I'm getting an error when running "make pep". Not sure what is going on, since I fixed the above issues. I don't see what the issue is, but maybe I'm missing something obvious.

@requires_numpydoc
   def test_docstring_parameters():
       """Test module docstring formatting."""
       from numpydoc import docscrape
   
       # skip modules that require mayavi if mayavi is not installed
       public_modules_ = public_modules[:]
       try:
           import mayavi  # noqa: F401 analysis:ignore
           public_modules_.append('mne.gui')
       except ImportError:
           pass
   
       incorrect = []
       for name in public_modules_:
           # Assert that by default we import all public names with `import mne`
           if name not in ('mne', 'mne.gui'):
               extra = name.split('.')[1]
               assert hasattr(mne, extra)
           with pytest.warns(None):  # traits warnings
               module = __import__(name, globals())
           for submod in name.split('.')[1:]:
               module = getattr(module, submod)
           classes = inspect.getmembers(module, inspect.isclass)
           for cname, cls in classes:
               if cname.startswith('_') and cname not in _doc_special_members:
                   continue
               with pytest.warns(None) as w:
                   cdoc = docscrape.ClassDoc(cls)
               for ww in w:
                   if 'Using or importing the ABCs' not in str(ww.message):
                       raise RuntimeError('Error for __init__ of %s in %s:\n%s'
                                          % (cls, name, ww))
               if hasattr(cls, '__init__'):
                   incorrect += check_parameters_match(cls.__init__, cdoc, cls)
               for method_name in cdoc.methods:
                   method = getattr(cls, method_name)
                   incorrect += check_parameters_match(method, cls=cls)
               if hasattr(cls, '__call__'):
                   incorrect += check_parameters_match(cls.__call__, cls=cls)
           functions = inspect.getmembers(module, inspect.isfunction)
           for fname, func in functions:
               if fname.startswith('_'):
                   continue
               incorrect += check_parameters_match(func)
       msg = '\n' + '\n'.join(sorted(list(set(incorrect))))
       if len(incorrect) > 0:
>           raise AssertionError(msg)
E           AssertionError: 
E           mne.io.base.BaseRaw.plot show_scrollbars != use_scalebars
E           mne.io.base.BaseRaw.plot use_scalebars != show_scrollbars
E           mne.io.base.Raw.plot show_scrollbars != use_scalebars
E           mne.io.base.Raw.plot use_scalebars != show_scrollbars
E           mne.io.base.RawArray.plot show_scrollbars != use_scalebars
E           mne.io.base.RawArray.plot use_scalebars != show_scrollbars
E           mne.viz.raw.plot_raw show_scrollbars != use_scalebars
E           mne.viz.raw.plot_raw use_scalebars != show_scrollbars

[codecov]

Codecov Report

Merging #7091 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #7091      +/-   ##
==========================================
+ Coverage   89.75%   89.75%   +<.01%     
==========================================
  Files         442      442              
  Lines       77788    77792       +4     
  Branches    12624    12625       +1     
==========================================
+ Hits        69815    69819       +4     
  Misses       5163     5163              
  Partials     2810     2810

[drammock]

This is from the Azure output:

E           mne.viz.raw.plot_raw : GL03 : Double line break found; please use only one blank line to separate sections or paragraphs, and do not leave blank lines at the end of docstrings
E           mne.viz.raw.plot_raw : PR03 : Wrong parameters order. Actual: ('raw', 'events', 'duration', 'start', 'n_channels', 'bgcolor', 'color', 'bad_color', 'event_color', 'scalings', 'remove_dc', 'order', 'show_options', 'title', 'show', 'block', 'highpass', 'lowpass', 'filtorder', 'clipping', 'show_first_samp', 'proj', 'group_by', 'butterfly', 'decim', 'noise_cov', 'event_id', 'show_scrollbars', 'use_scalebars', 'verbose'). Documented: ('raw', 'events', 'duration', 'start', 'n_channels', 'bgcolor', 'color', 'bad_color', 'event_color', 'scalings', 'remove_dc', 'order', 'show_options', 'title', 'show', 'block', 'highpass', 'lowpass', 'filtorder', 'clipping', 'show_first_samp', 'proj', 'group_by', 'butterfly', 'decim', 'noise_cov', 'event_id', 'use_scalebars', 'show_scrollbars', 'verbose')

I didn't realize that the internal parameter was called use_scalebar. I still think show_scalebar is a better name for the public API, so I'd be inclined to change the internal variable name to match the (new) public parameter name.

[cbrnr]

👍 for renaming to show_scalebar.

[drammock]

@adam2392 you're still failing the CI tests; FYI generally we wait to make sure all tests are passing before asking for review.

The first error I looked at on Azure was definitely related to this PR. Can you try to figure out what is failing, why it's failing, and fix it? If you try and fail, that's OK, just let us know that someone else needs to take over.

[adam2392]

Ah yes so sorry about that! I was running the tests locally and I think I didn't run the full suite. Will wait for the full CI to finish before doing so again.

[drammock]

cool. Note that you can run (from the root of the MNE-Python folder) pytest mne/viz/tests/test_epochs.py (for example) if you want to run only the tests in a single file. If your PR is a small one, this is usually sufficient to catch all the likely errors. I recommend this, actually, since the full test suite takes a very long time to run locally. See https://mne.tools/dev/install/contributing.html#running-the-test-suite

Please ping me again when all tests are passing and I'll review at that time.

[adam2392]

Thanks for the tips @drammock ! I am hoping to make more contributions to MNE throughout the future, as it's been such a huge help for my research.

Looks like all the tests have passed. Lmk if there is anything else you would like me to change.

[drammock]

I'm suggesting 2 changes, but marking as "approve" because I'm about to be unavailable for the rest of the day. Assuming you accept my 2 suggestions I think this is ready for merge.

[adam2392]

Cool, thanks for that quick catch! Yep, this looks good to me, and the plots look fine upon using it.

[larsoner]

Thanks @adam2392