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

[docs] clarify why add_config_value throws a type mismatch warning if type argument is omitted #11779

Open
arwedus opened this issue Nov 30, 2023 · 5 comments

Comments

@arwedus
Copy link

arwedus commented Nov 30, 2023

Describe the bug

Our custom extensions, which define config values for strings (actually path-strings) like shown below, fail to load when I switch from sphinx 6.1 to 7.2.6, with the following warning:

Warning, treated as error:
The config value `myextension_data_path' has type `str', defaults to `_StrPath'.

The extension's setup function looks like this:

def setup(app):
    app.add_config_value("myextension_data_path", app.srcdir, "html")

According to the docs, the types argument defaults to "Any", which I'd expect to mean that it accepts any type.

I saw some other people on the internet stumble upon this, and helped themselves like this:

scikit-hep/pyhf#2300

I've helped myself like this:

def setup(app):
    app.add_config_value("myextension_data_path", str(app.srcdir), "html")

What also works is this:

def setup(app):
    app.add_config_value("myextension_data_path", app.srcdir, "html", types=Any)

But both seem to me to be inconsistent with the documentation.

In any case, this seems to me to be an incompatible change between sphinx 6 and 7 that I could not find in the changelog. Please help me understand the rationale behind this.

How to Reproduce

I will provide a minimal example extension if you want.

Environment Information

Platform:              linux; (Linux-6.2.0-36-generic-x86_64-with-glibc2.35)
Python version:        3.10.12 (main, Nov 20 2023, 15:14:05) [GCC 11.4.0])
Python implementation: CPython
Sphinx version:        7.2.6
Docutils version:      0.18.1
Jinja2 version:        3.0.3
Pygments version:      2.16.1

Sphinx extensions

No response

Additional context

No response

@chrisjsewell
Copy link
Member

Heya app.srcdir has been changed to a pathlib.Path, instead of sphinx's bespoke path object (which happened to be a str subclass)

If you want to allow your users to provide either a string or a Path object, then off the top-of my head, I think it should be:

app.add_config_value("myextension_data_path", app.srcdir, "html", types=[str, Path])

@arwedus
Copy link
Author

arwedus commented Nov 30, 2023

@chrisjsewell : Undestood, but my point is, that if the types argument would default to Any as the documentation states, I should be allowed to provide any type, including Path, as default value without sphinx complaining.

@chrisjsewell
Copy link
Member

chrisjsewell commented Nov 30, 2023

the types argument would default to Any as the documentation states

I think you may be missreading this; the types argument is of type Any, and defaults to ().
Undocumented here, is that, if types=(), then it essentially changes this to types=(type(default),), i.e. the default type is that of the default value

@arwedus
Copy link
Author

arwedus commented Nov 30, 2023

Thanks for clarification, @chrisjsewell . I think the documentation should be extended by a note like this:

If types is omitted, the type of the default value will be used.

@picnixz picnixz changed the title add_config_value throws a type mismatch warning if type argument is omitted [docs] clarify why add_config_value throws a type mismatch warning if type argument is omitted Dec 24, 2023
@picnixz
Copy link
Member

picnixz commented Dec 24, 2023

PR for updating the doc is welcomed ! I'll leave the issue opened so that we know of one example.

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

No branches or pull requests

3 participants