Skip to content

Conversation

@lesteve
Copy link
Contributor

@lesteve lesteve commented May 29, 2024

make does not exist on Windows in general so spin docs does not work on Windows.

This small patch seems to be a reasonable way to make it more likely than spin docs works on Windows. I tested this on a Windows VM and it seems to work fine. The debug information is useful as well (i.e. cd doc is explicitly mentioned):

> spin docs html-noplot
$ export SPHINXOPTS=-W -j auto
$ cd doc
$ make.bat html-noplot

As noted in scikit-learn/scikit-learn#29012 (comment) make.bat is generated by sphinx-quickstart and some projects still have it. scikit-learn has it and I think it is supposed to work, although we don't have that many developers on Windows. numpy and matplotlib still have make.bat as well.

Something else that I may take a closer look at one point in scikit-learn would be to actually not use the Makefile at all for most useful doc-related functionalities and implement them as spin custom command. This is probably a bit of work (in particular doc/Makefile has some quirks related to environment variables in CI and different OS) and not high-priority right now.

@lesteve lesteve changed the title FIX make sphinx docs work under Windows FIX make sphinx docs work on Windows May 29, 2024
@stefanv
Copy link
Member

stefanv commented May 29, 2024

Surprisingly, it looks as though this works for bash shell on Windows too 👍 I think we should add one extra check: if make.bat exists, invoke it, otherwise run make.

@stefanv
Copy link
Member

stefanv commented May 30, 2024

Agreed that we should eventually fully replace the make/make.bat with the spin cmd, but this is a good fix for now; thanks!

@stefanv stefanv merged commit 9231958 into scientific-python:main May 30, 2024
@jarrodmillman jarrodmillman added this to the 0.11 milestone May 30, 2024
@lesteve lesteve deleted the windows-docs branch May 31, 2024 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: Enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants