| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
FYI it seems CI doesn't like editable installs, but I think it's fair not to have those in CI jobs anyway, hence manually installing dependencies in the workflows as opposed to leaning on the new make. |
Sorry, something went wrong.
|
I comment this on Slack (and also on the other PR), but let's please not require to install things to build the docs. It isn't necessary. My suggestion would be:
|
Sorry, something went wrong.
|
Not building against an installed version of the spec will also avoid potential confusion if someone has the release version of the spec installed and the docs build against that instead of the local dev version. There's no reason you'd ever want to build the docs against anything except for the local version. |
Sorry, something went wrong.
I had make do the make install part because of Ralf's comment making me concious of the longer invocation of the whole build process, but yes I see removing it makes sense with how make usually works. EDIT: done just that.
That doesn't make sense considering spec/ isn't a Sphinx doc folder now, and shouldn't be given how valuable the separate docs are—from #505: "The main drive in having technically separate docs is to keep utilising Sphinx's autodoc, which is incredibly useful but ill-equipped of having multiple spec versions in the repo. The ability to monkey-patch sys.modules in the conf.py of every version-doc gives us consistency, basically just so we don't have to worry about modifying the .. currentmodule targets for every version."
From #505 (comment): "I think actually we don't benefit much from magic here, as users still need to install the doc dependencies. Documenting in the README to use a single command pip install -e .[doc] doesn't seem too bad, even if just having to run pip install -r requirements.txt is more intuitive and what we're used to. Having sys.path.append("../../array_api_stubs/YYYY_MM/"); import YYYY_MM; sys.modules["array_api"] = YYYY_MM for all conf.py files isn't much a problem and I see not uncommon, but it's still something in terms of maintenance burden and environment hacks. Promoting just a single way to work on both the array_api_stubs package and the spec documentation feels a bit nicer IMO as it's more true-to-form for a Sphinx doc and doesn't confuse what the stubs actually are. Also I like the modern pyproject.toml process of the src/ folder which can contain multiple modules. Right now that's just _array_api_conf, which gets imported to each conf.py and saves us from path hacks per conf.py. Concretely I'd like to utilise src/ for a small Python script which can be run to build redirect pages in the future (unfortunately sphinx-reredirects isn't workable for what we need). Even if we don't do that specifically, it's nice to have the flexibility (i.e. the local development instructions won't change if we introduce a module)."
Yeah that's fair, although this is a potential problem when building most docs. |
Sorry, something went wrong.
There's a difference between installing a dependency which is just a standard Python package and installing some dev scripts.
I don't see how it's a maintenance burden. I use this pattern all the time in various docs and I never need to do anything with it. Also what is the sys.modules["array_api"] = ... bit for? Is that needed to trick autodoc? How is that different from installing the package? Alternatively you can set PYTHONPATH in the Makefile. The difference is that would not support running sphinx-build directly, but people generally shouldn't do that.
That's why I like the sys.path.insert(0, ...) pattern in conf.py. It ensures that you always build the docs against the local version, regardless of what is installed. |
Sorry, something went wrong.
This sounds like a useful pattern, and one that we've used in SciPy as well for a long time without any issues. |
Sorry, something went wrong.
i.e. so you don't have to explicitly install things to build docs
|
Pushed 19ad87c which uses sys.path
Yep to trick autodoc so generated docs and urls are prefixed with array_api, rather than if you say installed the package and would have {array_api_stubs.}_2021.12, {array_api_stubs.}_draft etc. (didn't seem to be a nice way to magic these generated module paths into array_api otherwise at the time of #505). Given this changes our GH/circleci workflows, I'll push this PR to honno#9 to see if at least the circleci preview works fine. |
Sorry, something went wrong.
There was a problem hiding this comment.
Sorry, something went wrong.
| "sphinx_copybutton", | ||
| "docutils<0.18", | ||
| "sphinx-math-dollar", | ||
| ] |
There was a problem hiding this comment.
Yep, this is unfortunately needed because pip is still not able to install from this dependency list without also installing the project itself.
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
README updated accordingly.
I'm not sure on general procedure with makefiles here: I assumed make install shouldn't be depended upon by the draft and spec commands.