Some changes in make.bat and the build documentation documentation by fredrik-1 · Pull Request #11121 · matplotlib/matplotlib

fredrik-1 · 2018-04-25T08:34:33Z

PR Summary

I changed the make.bat file so that it is possible to set command line options and wrote a more detailed instruction on how to build the documentation on windows.

(I am not really sure if everything works when building on windows though because I get some warnings but it works for my use)

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

timhoffm · 2018-04-25T09:30:13Z

Thanks! Looks good.

ImportanceOfBeingErnest · 2018-04-25T10:00:34Z

I do not understand the changes made here. Previously you'd just run make html and get the documentation out. Now you have to set the options manually?

fredrik-1 · 2018-04-25T10:19:56Z

You can still run just make html but you can also set some extra sphinx options, for example set -Dplot_gallery=0 to not run the scripts in the gallery.

There is one different though, the default was -W (make warnings to errors) before but this couldn't be changed on the command line for windows and I couldn't find a nice way to get the old behavior in the make.bat file.

ImportanceOfBeingErnest · 2018-04-25T10:37:13Z

So you are proposing to keep SPHINXOPTS=-W on linux, but make SPHINXOPTS= the default on windows?

ImportanceOfBeingErnest · 2018-04-25T10:52:39Z

+    workaround is to not build the gallery by commenting out 
+    ``sphinx_gallery.gen_gallery`` in ``extensions`` in the ``conf.py`` file 
+    or to delete the code in or delete the  
+    ``examples\userdemo\pgf_preamble_sgskip.py`` file.


This seems kind of a strange workaround. I'd say the workaround is to wait a few more days until 0.1.14 is finally released or install the current development version of sphinx-gallery.

fredrik-1 · 2018-04-25T11:03:24Z

That is how it is implemented now at-least and I don't know how to write the same default options functionality in the make.bat file as is done in the Makefile but it is of course possible to add a non sphinx option that resets the default. For example SPHINXOPTS=-W by default and SPHINXOPTS=clear -> SPHINXOPTS= in the sphinx call.

I think that SPHINXOPTS= is a better default in both though but I have very limited experience in building the documentation.

I am open to changing the work around but those two methods works good for my use right now and are simple to do.

anntzer

In agreement with @ImportanceOfBeingErnest: let's not document "delete the nonworking example" but rather let's wait for s-g 0.1.14. We can always revisit this if s-g 0.1.14 takes "too long" to get out.

ImportanceOfBeingErnest · 2018-04-26T16:03:38Z

So what about removing -W from make.bat? Is it acceptable to have different defaults for windows and linux? I think one would have to weight here: Is the default setting rather for people aiming at getting a local copy of the docs or at people trying to test if they build fine for submitting changes to matplotlib? Depending on that one or the other option may make more sense.

anntzer · 2018-04-26T21:26:02Z

That was discussed in #9513 (comment).

ImportanceOfBeingErnest · 2018-04-26T21:52:50Z

So over there the decision was to keep -W. (To which I would agree.)

What this means for this PR is that one would need to find a way to set this parameter via the command line; or leave make.bat as it currently is and let the user edit the file. I think this is fine as well; the drawback is then that you need to edit it back before each commit not to have it wandering around in your PR.

timhoffm · 2018-04-29T09:14:38Z

We could add some code that makes make.bat make SPHINXOPTS= html work. Doesn't neccessarily have to be part of this PR. Just keeping -W is good enough for the moment.

fredrik-1 · 2018-05-18T11:48:08Z

I changed the default to -W and made it possible to also change it on the command line.

anntzer · 2018-06-07T12:47:33Z

 * numpydoc>=0.4
 * Pillow
-* sphinx-gallery>=0.1.13
+* sphinx-gallery>=0.1.13 (>=0.2.0 on windows)


I would just document >=0.2.0 everywhere. The benefit of allowing 0.1.13 on non-Windows is minimal.

new version of s-g released

timhoffm · 2018-10-13T16:00:23Z

-* ``make O=-Dplot_gallery=0 html`` skips the gallery build.
+.. code-block:: sh
+
+    #runs a parallel build with 4 processes.


I'd go for # run a parallel build with 4 processes, i.e. use an imperative style rather than a statement, similar to how we write docstrings. The same holds for the following two comments.

Please also add a space between # and text (multiple places throughout the PR).

timhoffm · 2018-10-13T16:02:29Z

+Windows
+~~~~~~~
+
+The documentation is build using the ``make.bat`` file. The options are set using 


using ~~the~~ make.bat ~~file~~

timhoffm · 2018-10-13T16:05:14Z

+~~~~~~~
+
+The documentation is build using the ``make.bat`` file. The options are set using 
+environment variables and variables can be set either in the 


a bit less repetitive:

The options are controlled using environment variables. They can be set either in make.bat or on the command line before running make.bat.

timhoffm · 2018-10-13T16:13:39Z

+    #in cmd
+    #use the default -W option
+    make html 
+    set SPHINXOPTS=& make html


Can be left out. Or if you insist, put a # or line above.

timhoffm · 2018-10-13T16:19:02Z

+.. code-block:: sh
+
+    #in cmd
+    set SPHINXOPTS=-W


Can we use an example that is not the default value? IMHO that would make more sense.

timhoffm · 2018-10-13T16:22:49Z


-Multiple options can be combined using e.g. ``make O='-j4 -Dplot_gallery=0'
-html``.
+    #in powershell


PowerShell is a proper name, so we should capitalize it appropriately (multiple occurences throughout the PR).

timhoffm · 2018-10-13T16:35:20Z

-set SPHINXOPTS=-W
-set O=

+if "%SPHINXOPTS%" == "" (


Go for if not defined SPHINXOPTS https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/if.

That also makes it possible to use set SPHINXOPTS=& make html for setting an empty variable, i.e. we do not need the "set to anything but nothing" workaround.

timhoffm · 2018-10-13T16:39:43Z

I think the technical issues (sg) and discussion about the default are resolved.

This just needs a bit of cleanup.

jklymak · 2020-07-22T18:20:47Z

I'll close this as abandoned and probably obsolete, but feel free to reopen if someone is going to look at it again....

fredrik-1 force-pushed the doc_documentation branch from 9f28212 to c17c268 Compare April 25, 2018 08:52

dstansby added Documentation OS: Microsoft labels Apr 25, 2018

timhoffm approved these changes Apr 25, 2018

View reviewed changes

ImportanceOfBeingErnest reviewed Apr 25, 2018

View reviewed changes

anntzer previously requested changes Apr 26, 2018

View reviewed changes

fredrik-1 force-pushed the doc_documentation branch from c17c268 to 6753563 Compare April 26, 2018 08:20

fredrik-1 force-pushed the doc_documentation branch from 6753563 to aed84f6 Compare May 11, 2018 09:19

fredrik-1 force-pushed the doc_documentation branch from aed84f6 to 10a96be Compare May 18, 2018 10:02

fredrik-1 force-pushed the doc_documentation branch from 10a96be to 7fa5ddb Compare June 7, 2018 10:59

anntzer reviewed Jun 7, 2018

View reviewed changes

changes in building the documentation

c8c0129

fredrik-1 force-pushed the doc_documentation branch from 7fa5ddb to c8c0129 Compare June 8, 2018 16:16

timhoffm requested changes Oct 13, 2018

View reviewed changes

QuLogic added status: needs rebase status: needs revision labels Apr 1, 2020

jklymak closed this Jul 22, 2020

Uh oh!

Conversation

fredrik-1 commented Apr 25, 2018

PR Summary

PR Checklist

Uh oh!

timhoffm commented Apr 25, 2018

Uh oh!

ImportanceOfBeingErnest commented Apr 25, 2018

Uh oh!

fredrik-1 commented Apr 25, 2018

Uh oh!

ImportanceOfBeingErnest commented Apr 25, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fredrik-1 commented Apr 25, 2018

Uh oh!

anntzer left a comment

Choose a reason for hiding this comment

Uh oh!

ImportanceOfBeingErnest commented Apr 26, 2018

Uh oh!

anntzer commented Apr 26, 2018

Uh oh!

ImportanceOfBeingErnest commented Apr 26, 2018

Uh oh!

timhoffm commented Apr 29, 2018

Uh oh!

fredrik-1 commented May 18, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

timhoffm commented Oct 13, 2018

Uh oh!

jklymak commented Jul 22, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants