Skip to content
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.

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

MRG, DOC, ENH: new frequency-tagging tutorial #8867

Merged
merged 97 commits into from
Mar 4, 2021

Conversation

dominikwelke
Copy link
Contributor

@dominikwelke dominikwelke commented Feb 12, 2021

What does this implement/fix?

we propose a new tutorial for basic analysis of frequency-tagging data.
it comes with an example dataset from visual stimulation with inverting checkerboards.

Reference issue

no fixes, but loosely related to discussion about a new spectra class (#7671)

authors

@dominikwelke @kalenkovich

@dominikwelke dominikwelke changed the title MRG, DOC, ENH - new frequency-tagging tutorial MRG, DOC, ENH: new frequency-tagging tutorial Feb 12, 2021
@agramfort
Copy link
Member

thx @dominikwelke !

how big are the files? can we add this to the doc build to see it rendered on circleci?

@dominikwelke
Copy link
Contributor Author

hi @agramfort, thanks for the quick response!

how big are the files?

its about 40mb. i published the data on osf and the script fetches them from there

can we add this to the doc build to see it rendered on circleci?

locally it builds fine. how do i add it to the doc build?

@agramfort
Copy link
Member

if you call your tutorial plot_ssvep.py it will be automatically rendered on the website.
Can you try here while fixing the conflicting file?

thanks

@larsoner
Copy link
Member

@rob-luke @agramfort the latest rendering is here:

https://25353-1301584-gh.circle-artifacts.com/0/dev/auto_tutorials/time-freq/plot_ssvep.html

@dominikwelke can you add a conditional like https://github.com/mne-tools/mne-python/blob/main/tools/circleci_download.sh#L90-L92 ? It would get rid of the download inside the example rendering.

@dominikwelke
Copy link
Contributor Author

dominikwelke commented Feb 24, 2021

@dominikwelke can you add a conditional like https://github.com/mne-tools/mne-python/blob/main/tools/circleci_download.sh#L90-L92 ? It would get rid of the download inside the example rendering.

the conditional alone didnt work.. the ci sais InputError: (...) no such file or directory
i guess the dataset has to be listed somewhere else too. can you direct me to the spot?

@larsoner
Copy link
Member

the ci sais InputError: (...) no such file or directory

Fun! We've hit another release-related error, as PyQt5 5.15.3 came out 3 hours ago:

https://pypi.org/project/PyQt5/#history

I'll make a hotfix in main to pin us to !=5.15.3 and hopefully that fixes things for you

@larsoner
Copy link
Member

hope this worked

If you have all doc-building dependencies installed (e.g., with pip install -r requirements_doc.txt) and do:

$ cd doc
$ PATTERN=ssvep make html_dev-pattern
...
$ make show

it will build the docs plus your specific example. Locally when I do this on your branch and wait a few minutes for the built to complete I see:

Screenshot from 2021-02-24 15-49-15

So clearly I got the syntax wrong. :) Changing it to:

.. contents:: Outline
   :depth: 2

and re-running PATTERN=ssvep make html_dev-pattern produces:

Screenshot from 2021-02-24 15-53-24

@dominikwelke
Copy link
Contributor Author

dominikwelke commented Feb 24, 2021

If you have all doc-building dependencies installed

it worked fine locally but stopped working with the new theme (no theme named 'pydata_sphinx_theme' found (missing theme.conf?))
i'll update my environment

So clearly I got the syntax wrong. :) Changing it to:

.. contents:: Outline
   :depth: 2

great. i actually wondered whether this would be correct, but i blindly trusted you :)
changed

@rob-luke
Copy link
Member

do you have a good reference for this @rob-luke ?

Don't bother unless you can find one easily. Youve changed it to 15.0 now, so its clear to the reader. Thanks for the extra info. Its a great addition to the tutorials.

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.

Ok to merge when CIs are green (except the ones already red in main)

Thanks a lot @dominikwelke !

@dominikwelke
Copy link
Contributor Author

dominikwelke commented Mar 3, 2021

ok, all CIs are green @agramfort @larsoner @rob-luke @kalenkovich

I'll push a last commit with the changelog

Copy link
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

doc/changes/latest.inc Outdated Show resolved Hide resolved
tutorials/time-freq/plot_ssvep.py Outdated Show resolved Hide resolved
@kalenkovich
Copy link
Contributor

Other than two minor remaining issues, LGTM +1 for merge!

https://25558-1301584-gh.circle-artifacts.com/0/dev/auto_tutorials/time-freq/plot_ssvep.html

Committed your suggestions.

@agramfort
Copy link
Member

@dominikwelke can you merge main branch here to make sure CIs are green?

@dominikwelke
Copy link
Contributor Author

@dominikwelke can you merge main branch here to make sure CIs are green?

@agramfort all CIs are green now

@agramfort agramfort merged commit 4514ea7 into mne-tools:main Mar 4, 2021
@agramfort
Copy link
Member

thx a lot @dominikwelke and @kalenkovich !

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.

5 participants