Skip to content

MRG, ENH: add _repr_html_ method in the class BaseEpochs#9189

Merged
hoechenberger merged 13 commits intomne-tools:mainfrom
vagechirkov:add-repr-html-epochs
Mar 25, 2021
Merged

MRG, ENH: add _repr_html_ method in the class BaseEpochs#9189
hoechenberger merged 13 commits intomne-tools:mainfrom
vagechirkov:add-repr-html-epochs

Conversation

@vagechirkov
Copy link
Copy Markdown
Contributor

@vagechirkov vagechirkov commented Mar 23, 2021 •
edited
Loading

Reference issue #9177

Add _repr_html_ for BaseEpochs:

image

(updated image)

agramfort
agramfort reviewed Mar 24, 2021
View reviewed changes
Copy link
Copy Markdown
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.

the report should also be updated accordingly to use the template for -epo.fif files.

thx @vagechirkov

@agramfort
Copy link
Copy Markdown
Member

agramfort commented Mar 25, 2021

diff looks good

can you touch the plot_report file and maybe another one so we see in the doc artifact how it looks?

also don't forget to update what's new.

thx @vagechirkov

hoechenberger
hoechenberger reviewed Mar 25, 2021
View reviewed changes
hoechenberger
hoechenberger suggested changes Mar 25, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@hoechenberger hoechenberger left a comment

Choose a reason for hiding this comment

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

Thanks for your work on this! I left a few comments; looks good to me generally; I'd like to encourage you to make use of f-strings; also special-casing None in Epochs.baseline should not be necessary.

vagechirkov and others added 3 commits March 25, 2021 10:25
@vagechirkov @hoechenberger
Update mne/data/html_templates.py
241e4d0 
Co-authored-by: Richard Höchenberger <richard.hoechenberger@gmail.com>
@vagechirkov @hoechenberger
Update mne/data/html_templates.py
b441e5c 
Co-authored-by: Richard Höchenberger <richard.hoechenberger@gmail.com>
@vagechirkov @hoechenberger
Update mne/epochs.py
2e1100e 
Co-authored-by: Richard Höchenberger <richard.hoechenberger@gmail.com>
hoechenberger
hoechenberger reviewed Mar 25, 2021
View reviewed changes
@vagechirkov @hoechenberger
Update mne/data/html_templates.py
e2827c9 
Co-authored-by: Richard Höchenberger <richard.hoechenberger@gmail.com>
@vagechirkov
Copy link
Copy Markdown
Contributor Author

vagechirkov commented Mar 25, 2021

diff looks good

can you touch the plot_report file and maybe another one so we see in the doc artifact how it looks?

also don't forget to update what's new.

thx @vagechirkov

As far as I understood there are no epochs files in the example dataset. Here is the output of the report after I created one:

image

hoechenberger
hoechenberger reviewed Mar 25, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@hoechenberger hoechenberger left a comment

Choose a reason for hiding this comment

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

Looks very clean! Just one small suggestion.

@vagechirkov @hoechenberger
Update doc/changes/latest.inc
03752f1 
Co-authored-by: Richard Höchenberger <richard.hoechenberger@gmail.com>
hoechenberger
hoechenberger approved these changes Mar 25, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@hoechenberger hoechenberger left a comment •
edited
Loading

Choose a reason for hiding this comment

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

All good for me once CI goes green and artifact still looks okay!

@hoechenberger hoechenberger changed the title ENH: add _repr_html_ method in the class BaseEpochs MRG, ENH: add _repr_html_ method in the class BaseEpochs Mar 25, 2021
@hoechenberger hoechenberger merged commit a98c4f5 into mne-tools:main Mar 25, 2021
@hoechenberger
Copy link
Copy Markdown
Member

hoechenberger commented Mar 25, 2021

Thanks, @vagechirkov!

@vagechirkov vagechirkov deleted the add-repr-html-epochs branch March 25, 2021 17:11
@vagechirkov vagechirkov mentioned this pull request Mar 25, 2021
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@agramfort agramfort agramfort left review comments

+1 more reviewer

@hoechenberger hoechenberger hoechenberger approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vagechirkov @agramfort @hoechenberger