8000 Update to Sphinx-Gallery v0.1.11 by Titan-C · Pull Request #8222 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

Update to Sphinx-Gallery v0.1.11 #8222

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

Merged
merged 5 commits into from
Jun 6, 2017
Merged

Conversation

Titan-C
Copy link
Contributor
@Titan-C Titan-C commented Jan 22, 2017

This update includes the style change of using title to describe code links. This change was left pending in #7986 (comment)

It also make the gallery building robust on SyntaxErrors in the examples. Plus other enhancements not directly concerning Scikit-learn.

@Titan-C
Copy link
Contributor Author
Titan-C commented Jan 22, 2017

Travis is failing because sphinx extensions are tested. Last version of the gallery imports from sphinx but sphinx is not installed for testing in travis. Shall sphinx be installed or shall this test be ignored?

@lesteve
Copy link
Member
lesteve commented Jan 24, 2017

AFAIK doc/sphintext are third party extensions, I don't think we should test them.

Maybe historically we had more custom code in doc/sphinxext, I am not sure.

@jnothman
Copy link
Member
jnothman commented Jan 24, 2017 via email

@TomDLT
Copy link
Member
TomDLT commented Jan 25, 2017

#8228 is merged, if you update your PR with master, Travis should be green.

@Titan-C
Copy link
Contributor Author
Titan-C commented Jan 25, 2017

Rebased, travis is green

@Titan-C
Copy link
Contributor Author
Titan-C commented Jan 25, 2017

I figured it out about the download boxes being stacked. I comes from the responsive CSS layout and a mismatch in the width of the .container class from bootstrap. I could not make a better CSS, I just messed everything up. Thus my last commit gives the .sphx-glr-footer .container preference to have width: auto. This fixes the problem with the buttons being stacked and not in the center, without messing the complete CSS style of the scikit-learn website. It works well enough except for very wide screens, where the buttons are placed a bit more to the right.

@lesteve
Copy link
Member
lesteve commented Jan 30, 2017

I just pushed with [doc build] so we can look at the output in CircleCI.

@lesteve
Copy link
Member
lesteve commented Feb 28, 2017

@Titan-C I lost track of this one sorry. Is there anything else that needs to be done on the sphinx-gallery side?

@Titan-C Titan-C changed the title Update to Sphinx-Gallery v0.1.8 Update to Sphinx-Gallery v0.1.11 May 19, 2017
@Titan-C
Copy link
Contributor Author
Titan-C commented May 19, 2017

I'm bringing this back to life. Especially after the new Sphinx 1.6 release breaks the builds. There are some improvements from this version jump to the gallery 0.1.11.
Relevant changes are as stated in the first comment. The SyntaxError capturing and the new tooltip style. The other changes are concerned with how Sphinx-Gallery handles paths internally, and some bug fixes.

@lesteve
Copy link
Member
lesteve commented May 22, 2017

Thanks for reviving this one. Not sure whether the CircleCI failure was a glitch, I kicked off a rebuild.

@lesteve
Copy link
Member
lesteve commented May 22, 2017

Hmmm the example error seems like a genuine problem with the ichart.yahoo.com website ... From a quick googling the service seems to have been discontinued by Yahoo. For example see this link.

I guess we will need a work-around in the medium-term.

@Titan-C for this PR you could add plot_stock_market.py to the "expected to fail" examples.

The service this example relies on seems discontinued, thus we skip it as
an error. Once this is fixed, Sphinx-Gallery will raise the errors that the
script is not failing anymore.
doc/conf.py Outdated
'reference_url': {
'sklearn': None,
'matplotlib' 8000 : 'http://matplotlib.org',
'numpy': 'http://docs.scipy.org/doc/numpy-1.6.0',
'scipy': 'http://docs.scipy.org/doc/scipy-0.11.0/reference',
'nibabel': 'http://nipy.org/nibabel'}
'nibabel': 'http://nipy.org/nibabel'},
Copy link
Member

Choose a reason for hiding this comment

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

Not sure how nibabel got in there (probably from nilearn at one point) but we should remove it since nibabel is not used in scikit-learn examples.

@naoyak
Copy link
Contributor
naoyak commented May 22, 2017

Also related to Sphinx 1.6: #8828

@lesteve
Copy link
Member
lesteve commented May 23, 2017

The generated gallery is available here.

Can someone have a quick look to make sure there is nothing obviously wrong in the gallery and in the examples HTML? Maybe @jnothman @glemaitre

I think this PR should be merged because:

@glemaitre
Copy link
Member

I'm taking it a look at it.

@lesteve
Copy link
Member
lesteve commented May 23, 2017

Quickly looking at it, the only thing I noticed is that the download buttons are shifted to the right. Compare this PR and dev.

@Titan-C are the download buttons supposed to be centered?

@glemaitre
Copy link
Member

@lesteve I could not spot anything wrong related to the update apart of what you mentioned.

However, I could spot a lot of examples with:

####################
# some comments

It is usually used to split the code of the example. However, this is also the way the sphinx-gallery convert the example to notebook style. Therefore, some examples look a little bit messy. I would take some times to review the example and raise an issue.

@lesteve
Copy link
Member
lesteve commented May 23, 2017

I fixed the conflicts I introduced by merging #8921.

@lesteve
Copy link
Member
lesteve commented Jun 6, 2017

Let's merge this one!

@lesteve lesteve merged commit 7a23b24 into scikit-learn:master Jun 6, 2017
@lesteve
Copy link
Member
lesteve commented Jun 6, 2017

Thanks @Titan-C!

@lesteve
Copy link
Member
lesteve commented Jun 6, 2017

@Titan-C are the download buttons supposed to be centered?

@Titan-C do you have a comment about this?

@Titan-C
Copy link
Contributor Author
Titan-C commented Jun 10, 2017

@Titan-C are the download buttons supposed to be centered?

As I first wrote earlier in #8222 (comment). I don't know why download buttons started to appear stacked, but in this PR I included a fix to give them a nicer placement, but on big screens they look to be tilted to the right. Unfortunately to fix that you really need to mess with scikit-learn website layout a lot more.

@lesteve
Copy link
Member
lesteve commented Jun 10, 2017

OK thanks, I think this is fine for now.

Sundrique pushed a commit to Sundrique/scikit-learn that referenced this pull request Jun 14, 2017
@jnothman
Copy link
Member

We have been having Circle CI failures on master:

Exception occurred:
  File "/home/ubuntu/scikit-learn/doc/sphinxext/sphinx_gallery/gen_gallery.py", line 244, in setup
    if 'sphinx.ext.autodoc' in app._extensions:
AttributeError: 'Sphinx' object has no attribute '_extensions'

@glemaitre
Copy link
Member
glemaitre commented Jun 14, 2017 via email

@Titan-C
Copy link
Contributor Author
Titan-C commented Jun 14, 2017 via email

@lesteve
Copy link
Member
lesteve commented Jun 14, 2017

Hmmm I don't see any recent failures in https://circleci.com/gh/scikit-learn/scikit-learn.

@jnothman
Copy link
Member
jnothman commented Jun 14, 2017 via email

amueller added a commit to amueller/scikit-learn that referenced this pull request Jun 20, 2017
# Conflicts:
#	doc/sphinxext/sphinx_gallery/__init__.py
#	doc/sphinxext/sphinx_gallery/_static/gallery.css
#	doc/sphinxext/sphinx_gallery/docs_resolv.py
#	doc/sphinxext/sphinx_gallery/downloads.py
#	doc/sphinxext/sphinx_gallery/gen_gallery.py
#	doc/sphinxext/sphinx_gallery/gen_rst.py
#	doc/sphinxext/sphinx_gallery/notebook.py
#	doc/sphinxext/sphinx_gallery/py_source_parser.py
#	doc/themes/scikit-learn/static/nature.css_t
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Jun 20, 2017
dmohns pushed a commit to dmohns/scikit-learn that referenced this pull request Aug 7, 2017
dmohns pushed a commit to dmohns/scikit-learn that referenced this pull request Aug 7, 2017
NelleV pushed a commit to NelleV/scikit-learn that referenced this pull request Aug 11, 2017
paulha pushed a commit to paulha/scikit-learn that referenced this pull request Aug 19, 2017
AishwaryaRK pushed a commit to AishwaryaRK/scikit-learn that referenced this pull request Aug 29, 2017
maskani-moh pushed a commit to maskani-moh/scikit-learn that referenced this pull request Nov 15, 2017
jwjohnson314 pushed a commit to jwjohnson314/scikit-learn that referenced this pull request Dec 18, 2017
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.

6 participants
0