Skip to content

[MRG] deprecated/removed Sphinx config variables #8828

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 10, 2017
Merged

[MRG] deprecated/removed Sphinx config variables #8828

merged 1 commit into from
Oct 10, 2017

Conversation

naoyak
Copy link
Contributor

@naoyak naoyak commented May 4, 2017 •
edited
Loading

Updates the Sphinx config file to keep up with removals/deprecations in Sphinx 1.6.

  • exclude_trees -> exclude_patterns
  • latex_preamble -> latex_elements['preamble'] (currently emitting a deprecation warning on doc builds)

As far as I can tell the main issue is that the LaTeX preamble will not be picked up when rendering docs as PDF.

@naoyak naoyak changed the title DOC update Sphinx config variables [MRG] DOC update Sphinx config variables May 6, 2017
@naoyak naoyak changed the title [MRG] DOC update Sphinx config variables [MRG] DOC deprecated/removed Sphinx config variables May 18, 2017
@naoyak naoyak changed the title [MRG] DOC deprecated/removed Sphinx config variables [MRG] deprecated/removed Sphinx config variables May 18, 2017
@naoyak naoyak mentioned this pull request May 22, 2017
Merged
@naoyak naoyak force-pushed the sphinx-config branch 2 times, most recently from c947214 to e8fb176 Compare June 13, 2017 06:29
@naoyak
Copy link
Contributor Author

naoyak commented Jun 13, 2017

@lesteve one of the cfg variables affected here affects pdf doc generation...do you want to check this out?

@lesteve
Copy link
Member

lesteve commented Jun 14, 2017

Have you managed to get the pdf doc generation working with sphinx 1.6? I had to revert to sphinx 1.5 because there was an error I did not understand.

@lesteve
Copy link
Member

lesteve commented Jun 14, 2017

Also out of curiosity, does conf.py work with sphinx 1.5?

@jnothman jnothman added this to the 0.19 milestone Jun 14, 2017
@naoyak
Copy link
Contributor Author

naoyak commented Jun 14, 2017

Is it just make latexpdf to generate the pdf? I think conf.py should work with Sphinx 1.5 since the variables in question were already deprecated...

@naoyak
Copy link
Contributor Author

naoyak commented Jun 14, 2017

ok, I'm not quite able to figure out TeX setup on my Mac..

@amueller
Copy link
Member

I think this might need a rebase onto master with the new sphinxgallery to work with current sphinx.

@amueller amueller mentioned this pull request Jun 20, 2017
Closed
@amueller
Copy link
Member

I'm not sure we want to unpin sphinx entirely, given that it breaks so frequently (master seems broken right now, though going to 1.6.2 seems to fix it).

@lesteve
Copy link
Member

lesteve commented Jun 29, 2017 •
edited
Loading

#9227 has been merged so the pdf doc generation works with sphinx 1.6. @naoyak do you mind fixing the conflicts?

@naoyak naoyak force-pushed the sphinx-config branch 2 times, most recently from f417918 to a833e78 Compare June 29, 2017 08:20
@naoyak
Copy link
Contributor Author

naoyak commented Jun 29, 2017

thanks for the ping @lesteve - rebased now.

@naoyak
Copy link
Contributor Author

naoyak commented Jul 29, 2017

ping @lesteve

@naoyak naoyak force-pushed the sphinx-config branch 4 times, most recently from ac3b9c5 to c049bef Compare October 10, 2017 00:04
@naoyak
Copy link
Contributor Author

naoyak commented Oct 10, 2017

re-ping :)

@lesteve
Copy link
Member

lesteve commented Oct 10, 2017

As far as I can tell the main issue is that the LaTeX preamble will not be picked up when rendering docs as PDF.

I am not sure what you mean by this, can you elaborate?

Have you tried generating the PDF locally to quickly check that everything seemed fine?

Just curious, is it still compatible with sphinx 1.5?

@naoyak
Copy link
Contributor Author

naoyak commented Oct 10, 2017

@lesteve sorry, I don't think my laptop is setup to build LaTeX locally - I tried installing the packages a while back but couldn't quite get it to work.

regarding latex_preamble, I really have no experience with building LaTeX so I'm not sure if any of the specified packages/settings (

scikit-learn/doc/conf.py

Lines 227 to 230 in 71408e0

# Additional stuff for the LaTeX preamble.
latex_preamble = r"""
\usepackage{amsmath}\usepackage{amsfonts}\usepackage{bm}\usepackage{morefloats}
\usepackage{enumitem} \setlistdepth{10}
) are necessary for building the docs. the settings should be compatible with 1.5 as well.

@lesteve
Copy link
Member

lesteve commented Oct 10, 2017

OK thanks, I'll try to generate the pdf locally to see whether I can spot any obvious problem.

@lesteve
Copy link
Member

lesteve commented Oct 10, 2017 •
edited
Loading

I can generate the pdf locally and it looks fine. I rebased on master and pushed in your PR branch. Will merge when this is green.

@naoyak
Copy link
Contributor Author

naoyak commented Oct 10, 2017

Thanks @lesteve!

@lesteve
Copy link
Member

lesteve commented Oct 10, 2017

OK CircleCI is green, let's merge this one.

@lesteve lesteve merged commit 6b130f1 into scikit-learn:master Oct 10, 2017
@lesteve
Copy link
Member

lesteve commented Oct 10, 2017

Thanks a lot @naoyak!

@naoyak naoyak deleted the sphinx-config branch October 10, 2017 16:13
maskani-moh pushed a commit to maskani-moh/scikit-learn that referenced this pull request Nov 15, 2017
@naoyak @maskani-moh
MAINT: remove deprecated sphinx config variables (scikit-learn#8828)
feff0eb
jwjohnson314 pushed a commit to jwjohnson314/scikit-learn that referenced this pull request Dec 18, 2017
@naoyak
MAINT: remove deprecated sphinx config variables (scikit-learn#8828)
4d35478
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.19
Development

Successfully merging this pull request may close these issues.
4 participants
@naoyak @lesteve @amueller @jnothman