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

Boolean values passed to flag --define are wrongly interpreted as strings when building Sphinx docs #13273

Closed
bartvanb opened this issue Jan 28, 2025 · 1 comment · Fixed by #13318

Comments

@bartvanb
Copy link

bartvanb commented Jan 28, 2025

Describe the bug

I want to build Sphinx docs locally with --define autosummary_generate=0 to temporarily disable the autosummary extension. However, my value of 0 is not interpreted as boolean despite that the docs say I should provide 0 or 1 for booleans.

How to Reproduce

> sphinx-build docs docs/_build/html --define autosummary_generate=0
Running Sphinx v8.1.3
loading translations [en]... done
WARNING: The config value `autosummary_generate' has type `str'; expected `bool' or `list'.
loading pickled environment... The configuration has changed (1 option: 'pygments_dark_style')
done
WARNING: autosummary_generate: file not found: 0.rst

Note the WARNINGS: they show that sphinx-build does not interpret the 0 as I intended.

Relevant content conf.py:

extensions = [
    "sphinx.ext.autosummary",
]

autosummary_generate = True  # Turn on sphinx.ext.autosummary
@bartvanb bartvanb changed the title Values passed to flag --define are always interpreted as strings when building Sphinx docs Boolean values passed to flag --define are wrongly interpreted as strings when building Sphinx docs Jan 28, 2025
@AA-Turner
Copy link
Member

See also:


Extract from the thread in #13074:

From @timhoffm

Wouldn't it be better to coerce the type when processing the define parameter? By adding '0', '1' here, we (i) allow translation_progress_classes = '0' in conf.py, which seems unnecessarily permissive, and (ii) more importantly, every usage site has to check both truthy variants. - That's still okish in the present case, because there are only two usages, but it's not a pattern that scales for more heavily used parameters.

From me:

I think iff bool is a valid type for the config value, then 0 and 1 should be converted. I agree with Tim that this should ideally be done at the command-line-parsing stage, as we offer -D spam=0 as a shortcut to spam = False.

From @jayaddison (who's stepped back for the moment)

Ok, so to confirm: we'll extend the parsing of define options to check for corresponding config values, and for config values that contain either the bool type or an ENUM type that lists both bool values, we'll map the string values '0' and '1' to False and True respectively.

(maybe stating the somewhat-apparent-already, but just making sure -- the details about multiple types being possible for a config value, and also that ENUM can contain bool, are relevant here I think)


A

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants