Config: build.tools.python has to be a string to avoid confusions

[webknjaz]

Details

RTD build page presents an error message that doesn't make sense.

• Read the Docs project URL: https://readthedocs.org/projects/tomlkit-webknjazs-fork
• Build URL (if applicable):
  https://readthedocs.org/projects/tomlkit-webknjazs-fork/builds/18577928/
• Read the Docs username (if applicable): https://readthedocs.org/profiles/webknjaz/

Expected Result

Build should start.

Actual Result

It didn't. Instead, it errored out with a banner saying:

Problem in your project's configuration. Invalid "build.tools.python": expected one of (2.7, 3.6, 3.7, 3.8, 3.9, 3.10, 3.11, pypy3.7, pypy3.8, pypy3.9, miniconda3-4.7, mambaforge-4.10, 3), got 3.11

(emphasis mine)
The message says that 3.11 is not a valid version and lists 3.11 among valid versions. That is confusing. Also, the docs page at https://docs.readthedocs.io/en/stable/config-file/v2.html#sphinx-configuration says 3.11 is allowed.

The config file used:

---

# .readthedocs.yaml
# Read the Docs configuration file
# See https://docs.readthedocs.io/en/stable/config-file/v2.html
# for details

version: 2

sphinx:
  builder: dirhtml
  configuration: docs/conf.py
  fail_on_warning: true

build:
  os: ubuntu-22.04
  tools:
    python: 3.11

python:
  install:
  - method: pip
    path: .
  - requirements: docs/requirements.txt
  system_packages: false

...

[webknjaz]

Playing the guessing game, I could see it not allowing a float (the fix could be quoting it as '3.11') or the selected ubuntu version not actually having it (although, why would the validator running earlier emit that message?).

[webknjaz]

Aha! So it did want an explicitly quoted string. Why not just say that in an error message?

[humitos]

It seems that you find the problem yourself by adding quotes to the Python version. I agree the error message can be improved. Actually, I thought we had an issue about this, but I didn't find it now.

[stsewd]

I had this suggestion about this "problem" #8696 (comment).

[webknjaz]

I think it'd be reasonable to explicitly mention that the config value got parsed as a float and an explicit YAML string is expected.

[webknjaz]

Here's an idea: make a JSON schema published to SchemaStore. This would allow people to lint it in CI using https://github.com/python-jsonschema/check-jsonschema.git, plus many code editors would make use of it for autocomple. And finally, you could use the very same schema in RTD, making sure that it's the same for everyone.

[stsewd]

@webknjaz we already have one https://docs.readthedocs.io/en/stable/config-file/v2.html#schema, and we are already on schemastore. There is an open issue to use it to validate the config file on the server #8549.

[humitos]

At this point we could pass what's the expected data type to validate_choice so it mentions it in the message as well:

readthedocs.org/readthedocs/config/config.py

Lines 857 to 860 in 3ba5cbb

	build["tools"][tool] = validate_choice(
	version,
	self.settings["tools"][tool].keys(),
	)

[mvorisek]

In general, yaml is not strictly typed format and configs should accept unquoted numbers in places where only strings are allowed. Examples are Github Actions, Ansible, ...

If the RTD parser does not follow schema directly and parses the yaml context-free, it should not be that hard to post-process the parsed data based on the schema, ie. cast the types to based on the schema.

[webknjaz]

(with my Ansible Core Dev hat on)

@mvorisek it's not that simple. During parsing, the information about the initial input is lost (unless a custom parser is used). So an unquoted ambiguous 3.10 is parsed as a float 3.1, which will become a string "3.1" during post-processing. This is why so many apps require making such values explicit strings.
So while I agree that it looks nicer as a float, many people would be confused if they don't know the YAML's internals. This is typically solved by making the schema strict.

[humitos]

We've updated our error message to be more clear regarding this:

Would this solve/help with the confusion and guide the reader towards the solution?

[humitos]

I will close this issue since I think the new notification is clear enough. However, feel free to re-open or comment here if you consider there is anything else we can do to make it clearer.

[mvorisek]

With #11081 "latest" is supported.