-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
[MRG+1] CircleCI: run only modified examples in CircleCI #10407
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
Conversation
6d60f90
to
6760067
Compare
To be sure I get this right, this would only apply in the case where only examples are modified but if for instance a PR brings changes to the code of an estimator CircleCI would still run all the examples right? |
Current behaviour: when no examples is modified we just run I think you can force the full doc build by adding [doc build] in your commit message. I need to check that this is not affected by this PR by the way. |
Seems like a reasonable idea given the current policy, as long as it all remains intelligible |
6760067
to
90711dc
Compare
4b4fcdb
to
ebbc451
Compare
ebbc451
to
1eeab7f
Compare
I am making this MRG. I tested this:
|
They were changed to do some testing
doc/Makefile
Outdated
@@ -48,6 +48,16 @@ html-noplot: | |||
@echo | |||
@echo "Build finished. The HTML pages are in $(BUILDDIR)/html/stable." | |||
|
|||
html-pattern: | |||
# These two lines make the build a bit more lengthy, and the |
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.
But the second is commented out
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.
Copied and pasted from html
pattern (for better or worse) ...
doc/Makefile
Outdated
@@ -48,6 +48,16 @@ html-noplot: | |||
@echo | |||
@echo "Build finished. The HTML pages are in $(BUILDDIR)/html/stable." | |||
|
|||
html-pattern: |
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.
I'm not sure we need a new target here... Is it hard or problematic to just use the html target?
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.
I could not find a way to do it cleanly in the html target, suggestions more than welcome! The main reason: what should the default pattern be? If you set the default pattern in the Makefile, then it will override any change in conf.py
: the pattern is set on the command-line of the sphinx-build
command. I would expect conf.py
to have the last word.
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.
I haven't tried, but can't you do something like
ifdef EXAMPLE_RE
EXAMPLE_OPT := -d blah=$(EXAMPLE_RE)
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.
Outside of any target, that is
doc/developers/contributing.rst
Outdated
while. To save some time, you can use: | ||
- ``make html-noplot``: this will generate the documentation without the | ||
example gallery. This is useful when changing a docstring for example. | ||
- ``PATTERN=your_regex_goes_here make html-pattern``: only the examples |
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.
PATTERN -> EXAMPLES ?
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.
Hmmm OK this is a regex so I am not sure EXAMPLES is a great name. Maybe EXAMPLES_PATTERN although I liked that the target suffix and the environment variable matched. Full disclosure: I copied the names from mne-python ...
build_tools/circle/build_doc.sh
Outdated
echo BUILD: detected examples/ filename modified in $git_range: $(echo "$filenames" | grep -e ^examples/ | head -n1) | ||
echo BUILD: detected examples/ filename modified in $git_range: $changed_examples | ||
pattern=$(echo "$changed_examples" | paste -sd '|') | ||
echo "$pattern" |
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.
Deserves a comment
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.
Happy to add a comment but maybe you can tell about what exactly? Maybe that $pattern is a regex that decides which examples will be run? Note that I have a comment like this when build_type
is called below:
# pattern for examples to run is the last line of output
but maybe this is not enough?
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.
I just meant to explain why it's printing
Other minor changes including variable renaming PATTERN -> EXAMPLES_PATTERN
I reuse the html make target following your suggestion @jnothman. Also tackled your other comments. |
I will have a look now. |
LGTM. merging, Thanks @lesteve !!! |
Great idea from mne-python. This should reduce the CircleCI build times (from ~45 minutes to ~10 minutes + example running time) when modifying examples.
I got this from sphinx-gallery/sphinx-gallery#321 (comment).
Need to beta test the PR before this can be merged.