Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Some changes in make.bat and the build documentation documentation#11121
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Thanks! Looks good. |
I do not understand the changes made here. Previously you'd just run |
You can still run just There is one different though, the default was |
So you are proposing to keep |
doc/devel/documenting_mpl.rst Outdated
| 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. |
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.
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.
That is how it is implemented now at-least and I don't know how to write the same default options functionality in the I think that I am open to changing the work around but those two methods works good for my use right now and are simple to do. |
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.
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.
So what about removing |
That was discussed in#9513 (comment). |
So over there the decision was tokeep 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. |
We could add some code that makes |
I changed the default to |
doc/devel/documenting_mpl.rst Outdated
| @@ -47,7 +47,7 @@ requirements that are needed to build the documentation. They are listed in | |||
| * IPython | |||
| * numpydoc>=0.4 | |||
| * Pillow | |||
| * sphinx-gallery>=0.1.13 | |||
| * sphinx-gallery>=0.1.13 (>=0.2.0 on windows) | |||
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.
I would just document >=0.2.0 everywhere. The benefit of allowing 0.1.13 on non-Windows is minimal.
| * ``make O=-Dplot_gallery=0 html`` skips the gallery build. | ||
| .. code-block:: sh | ||
| #runs a parallel build with 4 processes. |
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.
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).
| Windows | ||
| ~~~~~~~ | ||
| The documentation is build using the ``make.bat`` file. The options are set using |
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.
usingthemake.batfile
| ~~~~~~~ | ||
| The documentation is build using the ``make.bat`` file. The options are set using | ||
| environment variables and variables can be set either in the |
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.
a bit less repetitive:
The options are controlled using environment variables. They can be set either in
make.bator on the command line before runningmake.bat.
| #in cmd | ||
| #use the default -W option | ||
| make html | ||
| set SPHINXOPTS=& make html |
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 be left out. Or if you insist, put a# or line above.
| .. code-block:: sh | ||
| #in cmd | ||
| set SPHINXOPTS=-W |
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 we use an example that is not the default value? IMHO that would make more sense.
| Multiple options can be combined using e.g. ``make O='-j4 -Dplot_gallery=0' | ||
| html``. | ||
| #in powershell |
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.
PowerShell is a proper name, so we should capitalize it appropriately (multiple occurences throughout the PR).
| if "%SPHINXOPTS%" == "" ( |
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.
Go forif not defined SPHINXOPTShttps://docs.microsoft.com/en-us/windows-server/administration/windows-commands/if.
That also makes it possible to useset SPHINXOPTS=& make html for setting an empty variable, i.e. we do not need the "set to anything but nothing" workaround.
I think the technical issues (sg) and discussion about the default are resolved. This just needs a bit of cleanup. |
I'll close this as abandoned and probably obsolete, but feel free to reopen if someone is going to look at it again.... |
PR Summary
I changed the
make.batfile 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