Skip to content

Accept type expressions in Parser.addini(type=...)#14751

Open
Pierre-Sassoulas wants to merge 8 commits into
pytest-dev:mainfrom
Pierre-Sassoulas:addini-type-expressions
Open

Accept type expressions in Parser.addini(type=...)#14751
Pierre-Sassoulas wants to merge 8 commits into
pytest-dev:mainfrom
Pierre-Sassoulas:addini-type-expressions

Conversation

@Pierre-Sassoulas

Copy link
Copy Markdown
Member

Prior refactor in order to be able to do #14692 cleanly.

addini now accepts plain Python types (str, bool, int, float) and unions of them (e.g. int | str) for its type argument, in addition to the existing string tags, as proposed by bluetech in #14675.

@Pierre-Sassoulas Pierre-Sassoulas added type: enhancement new feature or API change, should be merged into features branch type: refactoring internal improvements to the code skip news used on prs to opt out of the changelog requirement labels Jul 21, 2026
@Pierre-Sassoulas Pierre-Sassoulas removed the skip news used on prs to opt out of the changelog requirement label Jul 21, 2026
@psf-chronographer psf-chronographer Bot added the bot:chronographer:provided (automation) changelog entry is part of PR label Jul 21, 2026
@Pierre-Sassoulas
Pierre-Sassoulas force-pushed the addini-type-expressions branch from d9a3d92 to 412e21b Compare July 21, 2026 19:10

@bluetech bluetech left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, LGTM!

(As I mentioned in the previous comment, I also think Literal support would be nice for some configs, but that can be done separately).

Comment thread src/_pytest/config/argparsing.py Outdated
plain Python type.
"""
if isinstance(type_, str) and type_ in _INI_TYPE_TAGS:
return cast("_IniTypeTag", type_)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is the string type needed here (i.e. does replacing with _IniTypeTag not work)?

Comment thread src/_pytest/config/argparsing.py Outdated
FILE_OR_DIR = "file_or_dir"

#: The string tags accepted by :meth:`Parser.addini` for its ``type`` argument.
_IniTypeTag = Literal[

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's add : TypeAlias to these, then it will be automatically upgraded to type statement once that's available.

Comment thread src/_pytest/config/argparsing.py Outdated
if default is NOTSET:
default = get_ini_default_for_type(type)
if isinstance(ini_type, tuple):
# A union has no unambiguous implicit default; require an

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the exception message is clear enough that the comment is redundant.

@bluetech

Copy link
Copy Markdown
Member

I think this is a simple and clear enough extension to the existing mechanism that even if we ever go with something more elaborate like @RonnyPfannschmidt proposed, it won't be a bother.

Comment thread src/_pytest/config/argparsing.py Outdated
Comment on lines +228 to +230
type: _IniTypeTag
| type[bool | int | float | str]
| types.UnionType

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this can be written more type-safely as

type: _IniTypeTag | TypeForm[bool | int | float | str] | None = None,

where TypeForm is imported from typing_extensions under if TYPE_CHECKING (added in Python 3.15). It seems to work as intended in mypy at least.

@RonnyPfannschmidt

Copy link
Copy Markdown
Member

It's not blocking my ever evading elaboration 😅

@Pierre-Sassoulas
Pierre-Sassoulas marked this pull request as draft July 23, 2026 05:30
Pierre-Sassoulas added a commit to Pierre-Sassoulas/pytest that referenced this pull request Jul 23, 2026
A Literal of strings restricts an ini option's value to the given
choices, for example type=Literal["auto", "long"]. The choices are
shown in the --help output. Since the choices have no unambiguous
implicit default, an explicit default is required, as for unions.

Suggested by bluetech in the review of pytest-dev#14751.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Pierre-Sassoulas
Pierre-Sassoulas force-pushed the addini-type-expressions branch from 412e21b to 6c3ff25 Compare July 23, 2026 07:35
Comment thread src/_pytest/config/argparsing.py Outdated
Comment thread src/_pytest/config/argparsing.py Outdated
Samielakkad

This comment was marked as resolved.

Pierre-Sassoulas added a commit to Pierre-Sassoulas/pytest that referenced this pull request Jul 23, 2026
A Literal of strings restricts an ini option's value to the given
choices, for example type=Literal["auto", "long"]. The choices are
shown in the --help output. Since the choices have no unambiguous
implicit default, an explicit default is required, as for unions.

Suggested by bluetech in the review of pytest-dev#14751.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Pierre-Sassoulas
Pierre-Sassoulas force-pushed the addini-type-expressions branch from 47369e5 to 91dc674 Compare July 23, 2026 13:47
Pierre-Sassoulas and others added 6 commits July 23, 2026 16:23
The Literal of ini option type tags was repeated inline in
Parser.addini and get_ini_default_for_type. Name it, and name the
full form the type argument accepts, ahead of extending it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
addini now accepts plain Python types (str, bool, int, float) and
unions of them (e.g. `int | str`) for its `type` argument, in addition
to the existing string tags, as proposed by bluetech in pytest-dev#14675.

A union means the option accepts a value of any of its member types:
getini tries each member in order and returns the first that accepts
the value. In TOML config the native value may be of any member type,
and string-based formats (INI files, -o overrides) coerce it to the
first member that accepts it. Internally union types are normalized to
a tuple of the existing string tags, reusing the per-tag coercion;
single-tag registration and lookup are unchanged.

An invalid `type` argument raises ValueError instead of asserting,
since it is user (plugin) provided, and registering a union type
without an explicit default also raises ValueError instead of silently
defaulting to None.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Annotate the ini type aliases with TypeAlias
- Type addini(type=...) with TypeForm instead of type[...] | UnionType,
  which statically rejects unions of unsupported types
- Drop the unneeded string quoting in cast("_IniTypeTag", ...)
- Remove a comment redundant with its exception message

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A Literal of strings restricts an ini option's value to the given
choices, for example type=Literal["auto", "long"]. The choices are
shown in the --help output. Since the choices have no unambiguous
implicit default, an explicit default is required, as for unions.

Suggested by bluetech in the review of pytest-dev#14751.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Bruno Oliveira <bruno@soliv.dev>
Apply simplification review findings:

- Merge the two tag lookup tables in _ini_type_to_tag into one dict
  and a try/except, dropping the isinstance guards and the cast
- Compute get_origin(type) once in addini
- Add _ini_type_repr and use it for --help output and the getini
  error messages, instead of three inline formatting variants
  (Literal choices are now consistently rendered 'auto' | 'long')
- Move the Literal branch from _getini into _getini_value so type
  interpretation happens in one layer
- Merge the two versionadded 9.2 blocks in the addini docstring and
  drop the redundant second changelog sentence
- Tests: hoist the duplicated conftests to class constants,
  parametrize the invalid-type and Literal cases, and merge the two
  --help tests into one

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Pierre-Sassoulas
Pierre-Sassoulas force-pushed the addini-type-expressions branch from 57ce0f0 to 7f54d20 Compare July 23, 2026 14:33
@Pierre-Sassoulas
Pierre-Sassoulas marked this pull request as ready for review July 23, 2026 14:34
@Pierre-Sassoulas

Copy link
Copy Markdown
Member Author

Tried to cleanup the history while keeping it reviewable following the comments, I think it's ready now. I made the Litteral change I think it makes more sense to do everything at once.

@bluetech

Copy link
Copy Markdown
Member

Thanks for adding the Literal support!

I haven't reviewed that part yet, but just from a quick look, I think it doesn't support e.g. int | Literal["foo"]. I think some of our settings would want this (sorry I can't find it right now, I might be wrong).

Ideally, the support would be for any recursive combination of bool, int, float, Literal, str, union and list[...] thereof (the recursive list part might be problematic for ini backward compat though).

However the current support in the PR is fine and above compatible with it don't have to implement it in this PR.

The alias only exists under TYPE_CHECKING, so Sphinx cannot resolve
the reference in the rendered Parser.addini signature, failing the
docs build (-W with nitpicky).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Pierre-Sassoulas

Copy link
Copy Markdown
Member Author

There isn't a lot of additional work to do int | Literal["foo"], maybe you're thinking of verbosity (an int with Litteral["auto"] as a possible value, it's currently handled later in

global_level = self.getoption("verbose", default=0)
assert isinstance(global_level, int)
if verbosity_type is None:
return global_level
ini_name = Config._verbosity_ini_name(verbosity_type)
if ini_name not in self._parser._inidict:
return global_level
level = self.getini(ini_name)
if level == Config._VERBOSITY_INI_DEFAULT:
return global_level
return int(level)

@bluetech

Copy link
Copy Markdown
Member

@bluetech

Copy link
Copy Markdown
Member

Although for backward compat we will have to keep the str in this case and thus the Literal becomes redundant. But that's probably where I got the need for it.

Allows types such as int | Literal["auto"], where a value is accepted
if any member accepts it. This is the shape of several existing
options that take a number or a special string sentinel.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Pierre-Sassoulas

Pierre-Sassoulas commented Jul 23, 2026

Copy link
Copy Markdown
Member Author

Do we still need it for the future ? I pushed what I had. Imo Litteral is better than str in the case it can be used.

@bluetech bluetech left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks again!

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

Labels

bot:chronographer:provided (automation) changelog entry is part of PR type: enhancement new feature or API change, should be merged into features branch type: refactoring internal improvements to the code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants