-
Notifications
You must be signed in to change notification settings - Fork 950
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
Add group for doc test #1831
Add group for doc test #1831
Conversation
@fenwickipedia - The rationale for having the js and doc groups under just python 3.6 is that we'd like to not have to build those for every python version we are testing against. |
FYI I don't think 2.7 works for documentation and also 3.3. doesn't work with travis |
Do you mean the docs don't build in python 2.7? And do you mean that travis no longer supports 3.3? |
Yes |
Travis seems to be building these python 3.3 tests just fine. How about we simply add a 2.7 doc group, just like in https://github.com/jupyter-widgets/ipywidgets/blob/master/.travis.yml#L30-L31. |
That's odd I don't know why it's working for us and not for jupyter notebook |
@fenwickipedia i think you need to fix your gitprofile |
We need to test on 3.3, since ipywidgets 7 supports python 3.3. I think probably all you need to check is that docs build on 2.7, by adding just a single block like https://github.com/jupyter-widgets/ipywidgets/blob/master/.travis.yml#L30-L31. |
Thanks for the feedback folks! I'll do some scrubbing in Tuesday's sprint
per suggestions.
…-tf-
sent from my android phone
On Nov 20, 2017 6:34 PM, "Jason Grout" <notifications@github.com> wrote:
We need to test on 3.3, since ipywidgets 7 supports python 3.3. I think
probably all you need to check is that docs build on 2.7, by adding just a
single block like https://github.com/jupyter-widgets/ipywidgets/blob/
master/.travis.yml#L30-L31.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1831 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AKTPRxNiC1ITZqeVSXOOBq9jZH1p-LBLks5s4gx8gaJpZM4QlAX2>
.
|
@jasongrout I reverted changes and added 2.7 and it's failing now on the js builds for all versions:
|
@jasongrout I got your point about js doc builds and took them out of env: section, and added 3.3 back. (js build was failing previously because where I added js to env, I didn't add browser and other params). But builds are green now :D |
Thanks! This looks much simpler :). Why did we delete the |
Looking at it, it looks like deleting the |
.travis.yml
Outdated
@@ -26,10 +26,11 @@ script: | |||
- 'if [[ $GROUP == doc ]] ; then bash ./scripts/travis_script_doc.sh ; fi' | |||
- 'if [[ $GROUP == js ]] ; then bash ./scripts/travis_script_js.sh ; fi' | |||
matrix: | |||
include: |
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.
Can you put this line back in? I think deleting it was unintentional.
Typo, my bad! Pushed the fix. |
failed on the doc build stage, maybe just needs to be kicked again. |
Just putting a link here for the issue #1830 |
Looks great - thanks! I'll wait for the build to pass before merging. |
I'm seeing that 2.7 is built when the plan executes for docs, but in docs/source/conf.py we have:
which probably explains |
@blink1073 says regarding bugs in build of the notebooks in 2.7:
|
To elaborate on what @blink1073 is saying - the kernel issue indicates that the notebooks have a python 3 kernel specified, and it's not finding a python 3 kernel (hence the error). To proceed, it sounds like the thing to do is to get the docs rendering on python 2 first (i.e., set up a conda environment with python 2, and try making the docs). Fixing those errors will resolve these issues. I don't have a ton of experience with the nbsphinx and jupyter_sphinx plugins, which is what is being used to run the doc notebooks. It doesn't look like nbsphinx has a way to pass options on to nbconvert about what kernel to use (to use the option that @blink1073 pointed out above), so fixing this may involve either writing a something that manually converts the doc notebooks to use python 2 kernels, or extending nbsphinx to be able to pass options on to nbconvert (to run notebooks with a different kernel). Or we can say that docs build on python 3 only... |
P.S. I don't know of any complaints that our docs only build on python 3. To judge the impact of the effort above - are there situations where building docs on python 3 is a problem? |
jupyter/notebook#2808 has a good reference to building docs only having python 3 available. |
That brings us back to the status quo - the current repo only builds docs on python 3 anyway. So...no change? This PR is exposing the fact that building ipywidgets docs only works on python 3 right now. That's really good to know (thanks!). The question is whether it is worth it to fix it to build docs on python 2. |
It appears as the issue in the notebook pr has to do with not being able to call python 3 when testing. It may be useful to figure out how to build the docs if you're a python 2 contributor? |
I'll close this for now. If any developers on python 2 would like to push this through, please comment and we can reopen this or pursue it in a different PR. |
Test .travis.yml changes