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

Experimental: moving configuration to sphinx.toml file #14427

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

melissawm
Copy link

This is a draft PR to move all static docs configuration from the conf.py file to a toml file.

I did things in the simplest way possible since I figure it's no use trying to optimize this now.

cc @Carreau

This is a draft PR to move all static configuration from the conf.py
file to a toml file.
pyproject.toml Outdated
@@ -72,6 +72,7 @@ doc = [
"typing_extensions",
"exceptiongroup",
"ipython[test]",
"toml",
Copy link
Contributor

Choose a reason for hiding this comment

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

recommend tomli ; python_version<'3.11'

Copy link
Member

Choose a reason for hiding this comment

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

So something like this ?

Suggested change
"toml",
"toml",
"tomli ; python_version<'3.11'"

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, though with that trailing slash, and probably sorted 😊 .

Then the matching import tomllib/import tomli as tomllib try/except block where it's used.

@bollwyvl
Copy link
Contributor

bollwyvl commented May 1, 2024

On the main, reducing conf.py complexity is great. Putting another namespacing layer (without a schema), and a another file, on top of it... not so sure.

A crazy play I've recently considered is to take DRY, declarative project definition to a silly extreme, and unilaterally declare pyproject.toml#/tool/sphinx-j2 A Thing:

# pyproject.toml
[tool.sphinx-j2]
author = """{{ project['authors'][0]['name'] }}"""

...with the nickel workflow:

from pathlib import Path
import json
try:
    import tomllib
except ImportError:
    import tomli as tomllib
import jinja2

HERE = Path(__file__).parent
ROOT = HERE.parent
PPT = ROOT / "pyproject.toml"
PPT_DATA = tomllib.loads(PPT.read_text(encoding="utf-8"))

globals().update(
    json.loads(
        jinja2.Template(json.dumps(PPT_DATA["tool"]["sphinx-j2"], indent=2)).render(PPT_DATA)
    )
)   

# carry on with actual dynamic things... if needed

A complaint can be made about oh no, pyproject.toml's getting long... but it doesn't get shipped in .whl files, so I don't see much of a problem.

@Carreau
Copy link
Member

Carreau commented May 2, 2024

On the main, reducing conf.py complexity is great. Putting another namespacing layer (without a schema), and a another file, on top of it... not so sure.

Thanks @bollwyvl, this is for another project @melissawm and I are working on. We'd love to manage to move the configurations files of many projects to be declarative so that we can more generally analyse and send automatic update.

You can see that as the same as moving from setup.py to pyproject.toml, but for docs, and yes in the short term there will be another layer of indirection.

We also thinking of https://github.com/sphinx-toolbox/sphinx-pyproject and have seen that sphinx might also consider static configuration files.

@bollwyvl
Copy link
Contributor

bollwyvl commented May 2, 2024

Great! Doers get stuff done.

The sphinx issue (don't even want to link to it) is reminding me of the flake8 discussion around moving config to pyproject.toml.

Taking all the jinja stuff out of that stdlib-only strawman, so one did have to repeat some content, I'd still rather see that optionally managed in pyproject.toml#/tool/sphinx, with a proper schema, and as close as possible to the existing flat data model as possible.

# ... stdlib-only imports and parsing
globals().update(ppt['tool']['sphinx])
name = ppt['project']['name']

That third-party tool (which, while small, brings in a raft of other third party tools) seems like a mighty big hammer to achieve the same thing.

The sticky DX wickets I see are:

  • path resolution
    • writing in a file in / but all the paths would be resolved relative to /docs
  • the 5% left in python would have no type hints
    • but then we get all-but-no typehints in conf.py, so it's hardly worse

@Carreau
Copy link
Member

Carreau commented May 2, 2024

Yeah, schema and all we agree, and wether it's in pyproject.toml#/tool/sphinx, or if we want a more general pyproject.toml#/tool/docs for fields that are common to many tools is also goal, we just think that sphinx.toml as a first step, schema-less to acomodate the fact that sphinx – so far – is a bunch of everything, give the the opportunity to progressively go into pyproject.toml which should be stricter.

@bollwyvl
Copy link
Contributor

bollwyvl commented May 2, 2024

sphinx.toml

Sure, that file would always win, as it's more precise, and is much easier to get good DX from a single-purpose file with taplo in the loop.

pyproject.toml which should be stricter.

That's the magic of the Wild West tool namespace! Since there's all-but-no-way to validate it (the one on schemastore is... not fit for purpose at scale), you can do whatever you want! Indeed, the (evolving) strawman could give you arbitrary chunks, perhaps using TOML-ly - instead of Sphinx-y _:

globals.update(*ppt["tool"]["sphinx-with-arbitary-sections"].values())

which consumed:

[tool.sphinx-with-arbitrary-sections]
short-section = { some_key = "foo" } 

[tool.sphinx-with-arbitrary-sections.fat-section]
some_other_value = "bar"

This would yield a far more readable schema (and instance), and would handle the all-but-inevitable case of conflicting keys defined by different, un-namespaced extensions... and here the update would fail (i think) in that case. Neat!

tool/docs

Yeah... that's... extremely optimistic. Just spent a while in mkdocs and boy howdy, thou shalt not have any new opinions there, or be willing to do a lot of jumping jacks using... another magic-naming hooks system devoid of all typing assistance.

# relative to the source/ directory.
exclude_patterns = config["sphinx"]["exclude_patterns"]
# The name of the Pygments (syntax highlighting) style to use.
pygments_style = config["sphinx"]["pygments_style"]
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering if locals().update(config["sphinx"]) would work for most (all?) of that stuff.

for k, v in intersphinx_mapping.items():
intersphinx_mapping[k] = tuple(
[intersphinx_mapping[k]["url"], intersphinx_mapping[k]["fallback"]]
)
Copy link
Member

Choose a reason for hiding this comment

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

Obviously here we need some workaround, and updating locals won't work.

Copy link
Member

Choose a reason for hiding this comment

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

thought on publishing (and maintaining an up to date) intersphinx_mapping python package that just contain standard urls for most of the scientific Python stack ? This has no reasons to differ between proejcts right ?

Copy link
Member

Choose a reason for hiding this comment

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

I guess that works only if all the packages always point to latest/stable, but that woudl be a good default I think.

Comment on lines +61 to +70
[latex]
latex_documents = [
['index', 'ipython.tex', 'IPython Documentation',
'The IPython Development Team', 'manual', 'True'],
['parallel/winhpc_index', 'winhpc_whitepaper.tex',
'Using IPython on Windows HPC Server 2008',
"Brian E. Granger", 'manual', 'True']
]
latex_use_modindex = "True"
latex_font_size = "11pt"
Copy link
Member

Choose a reason for hiding this comment

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

I completely forgot we had latex/pdf output. I think we can/should nuke it.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, it's not building. I think we will be able to nuke all tex-specific configs.

Copy link
Author

Choose a reason for hiding this comment

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

I think keeping it for testing purposes may be helpful, tbh. Other projects still do rely on it so it's something to keep in mind long-term.

@bollwyvl
Copy link
Contributor

bollwyvl commented May 3, 2024

(not worried about the ipykernel vs bleeding-edge pytest fail)

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

3 participants