-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
[MRG] Example link #3346
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] Example link #3346
Conversation
|
Seems like it works now, but is very slow on circle. There is some issue on their site, so maybe I'll restart it later and see how it goes before we merge. |
|
Actually, now that I look at it, it looks like it was caching most of the time. Ready for review/merge. |
|
cool any idea why we don't have the thumbnails and just the default helmet? |
|
That's just the circle build. It doesn't run the examples. We should see them when this is merged |
|
let's see ! |
* 'master' of git://github.com/mne-tools/mne-python: [MRG] plot_ica_properties (mne-tools#3275) [MRG] Example link (mne-tools#3346) GUI (coregistration): scale step +/- instead of multiplicative (mne-tools#3345) Fix bug in set_bipolar_reference (mne-tools#3343) fixed spelling mistake (mne-tools#3341) Fixed spelling mistake (mne-tools#3339)
| examples_path = os.path.join(app.srcdir, "generated", "%s.examples" % name) | ||
| if not os.path.exists(examples_path): | ||
| # touch file | ||
| open(examples_path, 'w').close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you need to do this? If you are running latest sphinx-gallery it should not be necessary as they have lines like this in their code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See related issue:
sphinx-gallery/sphinx-gallery#126
And the fix that I committed that I think should make this unnecessary:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are they supposed to accomplish? Getting rid of build warnings? Those are gone already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, assuming that this is only to get rid of warnings we shouldn't need it, I'll commit to master
Takeover from #3344