Skip to content

ENH: Add webp support to Report #11359

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
larsoner merged 9 commits into from
Dec 7, 2022
Merged

ENH: Add webp support to Report #11359

larsoner merged 9 commits into from
Dec 7, 2022

Conversation

@larsoner
Copy link
Member

@larsoner larsoner commented Dec 7, 2022 •
edited
Loading

Saves ~50% in mne-bids-pipeline

$ python -c "import mne; print(mne.report.open_report('~/mne_data/derivatives/mne-bids-pipeline/ds000248_base/sub-01/meg/sub-01_task-audiovisual_report.h5'))"
<Report | 28 titles | 78 items | sub-01, task-audiovisual  | 31.2 MB
 Raw (original run 01), run 01                             |  1.0 MB
 Data quality                                              |  0.2 MB
 Raw (maxwell filtered), run 01                            |  0.9 MB
 Raw (filtered), run 01                                    |  0.8 MB
 Events                                                    |  0.0 MB
 Epochs: before cleaning                                   |  0.5 MB
 Epochs: after cleaning                                    |  0.5 MB
 Condition: Auditory                                       |  1.4 MB
 Condition: Visual                                         |  1.6 MB
 Condition: Auditory/Left                                  |  1.4 MB
 Condition: Auditory/Right                                 |  1.5 MB
 Contrast: Visual+Auditory                                 |  1.6 MB
 Contrast: Auditory/Right+Auditory/Left                    |  1.6 MB
 Decoding: full-epochs                                     |  0.0 MB
 Decoding: time-by-time                                    |  0.1 MB
 TFR Power: Auditory                                       |  0.0 MB
 TFR ITC: Auditory                                         |  0.1 MB
 TFR Power: Visual                                         |  0.0 MB
 TFR ITC: Visual                                           |  0.1 MB
 Noise covariance                                          |  0.6 MB
 BEM                                                       |  2.4 MB
 Sensor alignment                                          |  0.5 MB
 Forward solution                                          |  0.0 MB
 Source: Auditory                                          |  3.6 MB
 Source: Visual                                            |  3.7 MB
 Source: Auditory/Left                                     |  3.6 MB
 Source: Auditory/Right                                    |  3.6 MB
 Configuration file                                        |  0.0 MB
>

Closes #11358
Closes mne-tools/mne-bids-pipeline#682

@larsoner larsoner mentioned this pull request Dec 7, 2022
Closed
larsoner
larsoner commented Dec 7, 2022
View reviewed changes
mne/report/report.py
kwargs['pil_kwargs'] = dict(optimize=True, compress_level=9)
pil_kwargs.update(optimize=True, compress_level=9)
if pil_kwargs:
# matplotlib modifies the passed dict, which is a bug
Copy link
Member Author

@larsoner larsoner Dec 7, 2022

Choose a reason for hiding this comment

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

Opened matplotlib/matplotlib#24657 but in practice we won't ever really need to remove this line since it'll be very quick

@larsoner
Copy link
Member Author

larsoner commented Dec 7, 2022

Okay this PR now also:

  1. Removes image_format='gif' from Report, as it wasn't working anyway
  2. Sets image_format='auto' as the new default, which uses 'webp' if available and 'png' otherwise. I think this is okay as it should just end up being a change in practice that is invisible to the user other than a (potentially much) smaller file size

Ready for review/merge from my end @drammock

drammock
drammock approved these changes Dec 7, 2022
View reviewed changes
Copy link
Member

@drammock drammock left a comment

Choose a reason for hiding this comment

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

code changes look good. +1 for merge assuming the doc rendering looks right and CIs are green.

@larsoner larsoner mentioned this pull request Dec 7, 2022
Merged
1 task
agramfort
agramfort approved these changes Dec 7, 2022
View reviewed changes
Copy link
Member

@agramfort agramfort left a comment

Choose a reason for hiding this comment

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

❤️

@larsoner
Copy link
Member Author

larsoner commented Dec 7, 2022

CircleCI looks good, Azure is just a timeout, and using this PR's new default format 'webp' the reports inside mne-bids-pipeline's website build drop from 1.3 GB to 0.75 GB 🚀

@larsoner larsoner merged commit 8f6af80 into mne-tools:main Dec 7, 2022
@larsoner larsoner deleted the report branch December 7, 2022 21:42
larsoner added a commit to britta-wstnr/mne-python that referenced this pull request Dec 8, 2022
@larsoner
Merge remote-tracking branch 'upstream/main' into add_fwhm_morlet
c689886 
* upstream/main:
  ENH: Add webp support to Report (mne-tools#11359)
  ENH: More complete report repr (mne-tools#11357)
  MAINT: Simplify server installation instructions (mne-tools#11356)
  BUG: Fix where report replacement did not respect section (mne-tools#11346)
  [DOC] Fix video link for coregistration (mne-tools#11354)
larsoner added a commit to ealtamir/mne-python that referenced this pull request Dec 8, 2022
@larsoner
Merge remote-tracking branch 'upstream/main' into patch-1
5d2af8f 
* upstream/main:
  ENH: Add webp support to Report (mne-tools#11359)
  ENH: More complete report repr (mne-tools#11357)
  MAINT: Simplify server installation instructions (mne-tools#11356)
  BUG: Fix where report replacement did not respect section (mne-tools#11346)
  [DOC] Fix video link for coregistration (mne-tools#11354)
  Revert "[ENH] Add tutorial on time-frequency source estimation with STC viewer GUI" (mne-tools#11350)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@agramfort agramfort agramfort approved these changes

@drammock drammock drammock approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

BUG/ENH: Proper gif/anim support in Report MAINT: Website is huge

3 participants

@larsoner @agramfort @drammock