Skip to content

Cleanup doc/conf.py & local sphinx extensions #9708

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 2 commits into from
Feb 6, 2018

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Nov 6, 2017

Split out extensions to their own files.

Emit the latex symbol tables without breaks.

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@anntzer anntzer force-pushed the confpy branch 4 times, most recently from 08d3351 to cf0106d Compare November 6, 2017 23:27
@@ -1,173 +0,0 @@
"""
Copy link
Member

Choose a reason for hiding this comment

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

I am concerned that someone is depending on this. There is not much harm in carrying this file.

Copy link
Member

Choose a reason for hiding this comment

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

On second thought, was this even importable outside of our docs build? 🐑

Copy link
Member

Choose a reason for hiding this comment

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

The importable stuff is in lib/matplotlib/sphinxext.

@tacaswell
Copy link
Member

👍 conditional on gen_rst not being importable by anyone else.

@tacaswell tacaswell added this to the v2.2 milestone Nov 7, 2017
@anntzer
Copy link
Contributor Author

anntzer commented Nov 7, 2017

I can't see how it would be. For example it is not included in a (default) wheel.
(Note that the conf.py explicitly has

# If your extensions are in another directory, add it here. If the directory
# is relative to the documentation root, use os.path.abspath to make it
# absolute, like shown here.
sys.path.append(os.path.abspath('.'))

which is what made it importable in the docs build.)

doc/conf.py Outdated
'matplotlib.sphinxext.plot_directive',
'sphinxext.github',
'sphinxext.math_symbol_table',
'sphinxext.mock_gui_toolkits',
Copy link
Member

Choose a reason for hiding this comment

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

If we drop support for sphinx older than 1.3 we can probably replace this with the build in autodoc_mock_imports in http://www.sphinx-doc.org/en/stable/ext/autodoc.html#confval-autodoc_mock_imports If we drop versions below 1.6 we can make it simpler by only mocking the top level modules. As I recall it I added the mocking here predates Sphinx 1.3 which is why its done manually

Copy link
Member

Choose a reason for hiding this comment

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

What is the argument for not requiring 1.6?

Copy link
Member

Choose a reason for hiding this comment

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

The only one I can think of is building with Sphinx from apt-get/yum but I am not sure we need to support that

Copy link
Member

Choose a reason for hiding this comment

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

I think debian does build our docs as part of the build process. @sandrotosi How much pain would it cause you to bump to sphinx 1.6?

Copy link
Contributor

Choose a reason for hiding this comment

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

it would be no problem at all, actually it would be really helpful as in Debian we're migrating to sphinx 1.6 and the current doc cannot be built with that version, so thanks for pushing to use 1.6 :)

Copy link
Contributor

Choose a reason for hiding this comment

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

nope, what i really should do is updating matplotlib in debian (which i'll do as soon as i find time for it)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think that's going to work because we need some mocked objects to be types (so that we can subclass them) or callables with specific return values (sip.getapi), rather than just "existing" (in other words, because we are running code at the toplevel rather than just defining a bunch of functions).

Copy link
Member

Choose a reason for hiding this comment

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

Debian 7 “Wheezy” is still supported (https://wiki.debian.org/LTS) and it has 1.1.3 sphinx (https://packages.debian.org/search?keywords=python-sphinx) =\

Copy link
Contributor

Choose a reason for hiding this comment

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

TBH i wouldnt worry about wheezy (nor jessie) - if anyone will want to get an updated mpl on that release, they'll have to take care of backporting a tons of other deps (numpy in primis) before caring about sphinx and missing documentation

Copy link
Member

Choose a reason for hiding this comment

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

True, but even stretch does not have 1.6

@anntzer
Copy link
Contributor Author

anntzer commented Nov 7, 2017

I think the mocking would be better resolved in a separate PR that strips out all the backend_foo individual doc pages (which are incomplete anyways) in favor of a single, hand-written doc for all the backends that lists the common public API of all backends.

@anntzer
Copy link
Contributor Author

anntzer commented Jan 1, 2018

Also got rid of the mpl_examples symlink, which is now unneeded.

@QuLogic
Copy link
Member

QuLogic commented Jan 9, 2018

A tangentially related request; can you move the sphinx_gallery import to after check_deps so sphinx doesn't fail with an import error instead of our proper error message?

@anntzer
Copy link
Contributor Author

anntzer commented Jan 9, 2018

done

@jklymak
Copy link
Member

jklymak commented Jan 19, 2018

This needs a rebase and tests to pass....

@anntzer anntzer force-pushed the confpy branch 2 times, most recently from ae39a7a to 9597c9a Compare January 20, 2018 09:31
@anntzer
Copy link
Contributor Author

anntzer commented Jan 20, 2018

done (well, OSX is backlogged...)

doc/conf.py Outdated
# Use IPython's console highlighting by default
extensions.extend(['IPython.sphinxext.ipython_console_highlighting',
'IPython.sphinxext.ipython_directive'])
from sphinx_gallery.sorting import ExplicitOrder
Copy link
Member

Choose a reason for hiding this comment

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

Why move the import from the head of the file here?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Thanks. IMO this could use a short comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

This is rendered as :doc:`/tutorials/introductory/customizing` (see the
bottom of the page. Note that this is in a tutorial; See
:ref:`writing-examples-and-tutorials` below)
This is rendered as at the bottom of
Copy link
Member

Choose a reason for hiding this comment

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

as

reference when the documentation is built.
Note that the python script that generates the plot is referred to, rather than
any plot that is created; sphinx-gallery will provide the correct reference
when the documentation is built.
Copy link
Member

Choose a reason for hiding this comment

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

I would leave the . instead of the ;

@QuLogic
Copy link
Member

QuLogic commented Feb 4, 2018

Needs a rebase now, too.

@anntzer anntzer force-pushed the confpy branch 2 times, most recently from 55ce5fd to 8f4fd73 Compare February 6, 2018 08:16
@anntzer
Copy link
Contributor Author

anntzer commented Feb 6, 2018

done

Emit the latex symbol tables without breaks.
@timhoffm timhoffm merged commit b7d87d9 into matplotlib:master Feb 6, 2018
@anntzer anntzer deleted the confpy branch February 6, 2018 18:26
@QuLogic QuLogic modified the milestones: needs sorting, v2.2.0 Feb 12, 2018
@tacaswell tacaswell mentioned this pull request Apr 30, 2018
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants