Skip to content

Conversation

@hoechenberger
Copy link
Member

What does this implement/fix?

When using Report.parse_folder(), including the "base directory" (i.e., folder name) in all captions's unnecessarily redundant and impairs legibility. This commit changes the behavior of Report such that when parse_folder() is used, the base directory name will be stripped from the captions.

master

Screenshot 2020-07-09 at 17 43 10

PR branch

Screenshot 2020-07-09 at 17 42 39

When using `Report.parse_folder()`, including the "base directory"
(i.e., folder name) in all captions's unnecessarily redundant
and impairs legibility. This commit changes the behavior of `Report`
such that when `parse_folder()` is used, the base directory name will be
stripped from the captions.
@hoechenberger hoechenberger changed the title No data_dir in captions when using parse_folder Strip base directory name from Report captions when using parse_folder Jul 9, 2020
@larsoner
Copy link
Member

larsoner commented Jul 9, 2020

Can you touch auto_tutorials/misc/plot_report.html so that we can see how it looks on CircleCI?

@hoechenberger
Copy link
Member Author

Can you touch auto_tutorials/misc/plot_report.html so that we can see how it looks on CircleCI?

Good idea

###############################################################################
# With the context manager, the updated report is also automatically saved
# back to :file:`report.h5` upon leaving the block.
# This line needs to be removed.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI instead of doing this, which will require another commit (and another round of CIs), it's better to either improve the wording/tutorial in some way, or (if you can't easily find an improvement) do some harmless (PEP8-compatible) newline removal or insertion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also in a commit where all you care about is Circle running, it helps to do [skip travis] in the commit message since Travis is usually our CI bottleneck (and Azure doesn't have an equivalent)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @larsoner, I will note this down :)

@larsoner
Copy link
Member

larsoner commented Jul 9, 2020

@hoechenberger hoechenberger changed the title Strip base directory name from Report captions when using parse_folder [MRG] Strip base directory name from Report captions when using parse_folder Jul 9, 2020
@agramfort agramfort merged commit 6bfa9c8 into mne-tools:master Jul 9, 2020
@agramfort
Copy link
Member

thx @hoechenberger

@hoechenberger hoechenberger deleted the report-title branch July 9, 2020 20:07
larsoner added a commit to larsoner/mne-python that referenced this pull request Jul 14, 2020
* upstream/master: (21 commits)
  MRG: Add SSP projectors to Report (mne-tools#7991)
  BUG: Fix warning (mne-tools#8006)
  WIP: TFR Doc/test changes (mne-tools#7998)
  MAINT: Remove numpydoc test on 3.6 (mne-tools#8005)
  MAINT: Better error message for mismatch (mne-tools#8007)
  MRG: Allow removal of active projectors if channels they applied to have meanwhile been dropped (mne-tools#8003)
  MRG Freesurfer coordinate frame tutorial (mne-tools#7578)
  FIX: Fix stockwell checks (mne-tools#7996)
  MRG, ENH: Add array-spacing plugin and reorganize deps (mne-tools#7997)
  MRG, ENH: Reduce memory usage of Welch PSD (mne-tools#7994)
  STY: One more [ci skip]
  STY: Docstyle [ci skip]
  Report parsing (mne-tools#7990)
  MRG, ENH: BrainVision impedance parsing (mne-tools#7974)
  BUG: Fix missing source space points (mne-tools#7988)
  [MRG] Strip base directory name from Report captions when using parse_folder (mne-tools#7986)
  DOC: Update estimates [skip travis] (mne-tools#7987)
  DOC: Try to improve find_bad_channels_maxwell doc (mne-tools#7982)
  VIZ, BUG: fix tickmarks in evoked topomap colorbar (mne-tools#7980)
  Add low-pass filter to find_bad_channels_maxwell() (mne-tools#7983)
  ...
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.

3 participants