Skip to content
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

Add notes from 2022-10 #61

Merged
merged 3 commits into from
Oct 17, 2022
Merged

Add notes from 2022-10 #61

merged 3 commits into from
Oct 17, 2022

Conversation

encukou
Copy link
Member

@encukou encukou commented Oct 4, 2022

No description provided.

@encukou
Copy link
Member Author

encukou commented Oct 4, 2022

The build is broken due to executablebooks/MyST-Parser#612 :(


* Allows single source of truth for definitions, etc.
* Not offline buildable
* Petr: would like Sphinx to provide a directory to reference local inv files to do offline builds
Copy link

@bskinn bskinn Oct 4, 2022

Choose a reason for hiding this comment

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

If I understand what the goal is here, this is already possible. When you declare intersphinx_mappings in conf.py, you can pass a tuple to the second argument. Sphinx will walk through each element of the tuple and work with the first successfully-found objects.inv. (You can still use None as one of the tuple elements to check the default web location.)

See "Multiple targets for the inventory" in the intersphinx docs.

Copy link
Member

Choose a reason for hiding this comment

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

This is what we talked about, but Petr apparently wanted to propose something a little different.

Copy link

@bskinn bskinn Oct 4, 2022

Choose a reason for hiding this comment

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

Interesting...I'm curious now what he's wanting.

Just about anything beyond that that I can think of should be handle-able by some dynamic code in conf.py.

Copy link
Member Author

Choose a reason for hiding this comment

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

The distro doesn't really “own” conf.py, and would have to patch it. That's ugly and fragile/tedious. (Also note that when building docs for offlline use, we want the Intersphinx links to point to other offline (locally installed) docs, rather than to the Web.)
So I would prefer an external way (e.g. file named by environment variable) to specify that a given URL should be replaced by a local file. But that sounds like a “selfish” request from a distro, so I'm wary of suggesting it before I have time to contribute an implementation.
However, it could pave the way to support for building & interlinking multiple docs projects locally, for offline use: if projects could specify the “publish URL” in conf.py, Sphinx could also update the URL→file mapping. Perhaps that would be useful more widely.

Copy link

Choose a reason for hiding this comment

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

Nod, I see what you mean -- the need is potentially general, so better for it to be an official Sphinx feature rather than a conf.py hack; and, better just overall for it not to be a conf.py hack.

Thanks for the details!

Copy link
Member

Choose a reason for hiding this comment

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

I, I see. That makes more sense now.

As a short term approach, we could just read in some env variable in the conf.py and use that to set the file path—it isn't ideal, but it isn't that hacky. Certainly long term I imagine it would be nice to have a better mechanism, but the timeline for distros adopting a Sphinx version with it might not be short or consistant with adopting the relevant Python version(s) with Intersphinx.

if projects could specify the “publish URL” in conf.py, Sphinx could also update the URL→file mapping.

Hmm, though projects would ultimately still need a way to find the other projects' conf.pys, and have some sort of mapping to them that the original project can specify. I imagine that's possible on the distro side, but I'm not sure if there's a generalized solution beyond that.

Copy link
Member

Choose a reason for hiding this comment

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

FYI, see python/cpython#97781 for the initial motivating case

Copy link
Member

Choose a reason for hiding this comment

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

Thinking about it some more, just being able to override intersphinx_mapping with some arbitrary locally set value would give you both things you want—you can set both the source .inv location and the target of the generated links to resolve to a local file location as well as a HTTP(S) URL. Right now, we could add a bit of code in conf.py that reads this value in from an env variable if present and evals it, overriding the configured default.

A longer term Sphinx-level solution could perhaps be some way, e.g. a config file, to map URLs to source .invs and target docs locations.

Copy link
Member

Choose a reason for hiding this comment

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

I'll open an issue, PR and Discourse thread to summarize all this and continue discussion, as soon as I get a free moment during the sprints.

Copy link
Member Author

Choose a reason for hiding this comment

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

IMO:
For the motivating example: if Python wants to adopt PyPA terminology in its own docs, we should copy the definitions over.
For distros (speaking with my Python hat on): don't worry too much about them. If they care, they can patch/workaround, and then iterate toward a better solution.

@CAM-Gerlach
Copy link
Member

Seems like @hugovk fixed the build, if you want to rebase or merge this to pick up the fix.

Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

A few small tweaks

docs/monthly-meeting/2022-10.md Outdated Show resolved Hide resolved
docs/monthly-meeting/2022-10.md Show resolved Hide resolved
docs/monthly-meeting/2022-10.md Outdated Show resolved Hide resolved
docs/monthly-meeting/2022-10.md Outdated Show resolved Hide resolved
* Docs to support Sphinx 5.x. @CAM-Gerlach wants to add a CI workflow during the sprint week for the oldest-supported version of Sphinx.
* Adam will work on sphinx support on python-docs-theme

* (Ezio) Should we enable the default role and alias it to `:literal:` (`` `...` ``) for docs.python.org?
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* (Ezio) Should we enable the default role and alias it to `:literal:` (`` `...` ``) for docs.python.org?
* (Ezio) Should we enable the default role (`` `...` ``) for docs.python.org?

Copy link
Member Author

Choose a reason for hiding this comment

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

As I understand it, the default role is already enabled, it's just aliased to... something that shows up as italics.
So it would be more correct this way?

Suggested change
* (Ezio) Should we enable the default role and alias it to `:literal:` (`` `...` ``) for docs.python.org?
* (Ezio) Should we alias the default role (`` `...` ``) to `:literal:` for docs.python.org?

Copy link
Member

Choose a reason for hiding this comment

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

Indeed "enable" might not be the best word. What I meant is "allow the use of the default role", not actually enabling it in the code or configuration.

That's the first step. If we do decide to allow it, then the next step is bikeshedding on what the best aliasing would be. I tried to better convey this by separating the two steps in this section.

Copy link
Member Author

Choose a reason for hiding this comment

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

To me, it dosesn't make sense to allow using it before we decide what it should mean...
Anyway, let's keep the original wording? These are meeting notes, not a specification -- it can be imprecise.

Copy link
Member

Choose a reason for hiding this comment

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

Just to note, for now, @hugovk fixed current usages and had Sphinx-Lint flag future ones in python/cpython#97998 (which I'm currently working to backport to 3.11 and 3.10).

docs/monthly-meeting/2022-10.md Outdated Show resolved Hide resolved
docs/monthly-meeting/2022-10.md Outdated Show resolved Hide resolved
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Co-authored-by: Ezio Melotti <ezio.melotti@gmail.com>
Co-authored-by: Ezio Melotti <ezio.melotti@gmail.com>
@encukou encukou merged commit d47b714 into python:main Oct 17, 2022
@encukou encukou deleted the 2022-10 branch October 17, 2022 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants