Accept type expressions in Parser.addini(type=...)#14751
Accept type expressions in Parser.addini(type=...)#14751Pierre-Sassoulas wants to merge 2 commits into
Conversation
Pierre-Sassoulas
added
type: enhancement
Pierre-Sassoulas
removed
the
skip news
psf-chronographer
Bot
added
the
bot:chronographer:provided
Pure refactor: move the ini/toml mode dispatch into a helper method, in preparation for calling it per union member when addini grows support for union types. 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>
d9a3d92 to
412e21b
Compare
bluetech
left a comment
bluetech
left a comment
There was a problem hiding this comment.
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).
| plain Python type. | ||
| """ | ||
| if isinstance(type_, str) and type_ in _INI_TYPE_TAGS: | ||
| return cast("_IniTypeTag", type_) |
There was a problem hiding this comment.
Is the string type needed here (i.e. does replacing with _IniTypeTag not work)?
| FILE_OR_DIR = "file_or_dir" | ||
|
|
||
| #: The string tags accepted by :meth:`Parser.addini` for its ``type`` argument. | ||
| _IniTypeTag = Literal[ |
There was a problem hiding this comment.
Let's add : TypeAlias to these, then it will be automatically upgraded to type statement once that's available.
| 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 |
There was a problem hiding this comment.
I think the exception message is clear enough that the comment is redundant.
|
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. |
| type: _IniTypeTag | ||
| | type[bool | int | float | str] | ||
| | types.UnionType |
There was a problem hiding this comment.
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.
|
It's not blocking my ever evading elaboration 😅 |
3 participants
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 itstypeargument, in addition to the existing string tags, as proposed by bluetech in #14675.