[svlandeg]

Meta: marked as a "feature", but definitely also "breaking" !

Description

Make Pydantic a required dependency and rely on it for parameter validation. Picked >=2.5.3 which is the current pin for github-actions.

Click's ParamType hierarchy is now replaced by a three-layer design:

• TypeDescriptor objects store static facts about a parameter's type (in the new module coercion.py)
• Pydantic TypeAdapter objects are defined in the new module adapters.py
• RuntimeParam subclasses own coercion (also defined in coercion.py)

Further type-specific functionality is bundled in the new module param_types.py.

How to review this

• Probably read all of the below first 👇
• Note that _click/types.py and typer/_types.py are deleted
• First look at typer/param_types.py, then typer/adapters.py, then typer/coercion.py
• Look at the changes in typer/core.py
• Check all other changes in typer/*.py
• Check all changes in typer/_click/*.py
• Check all changes in the test suite & documentation

Extended/improved functionality

• More date time formats are now supported by default, instead of just the three [%Y-%m-%d|%Y-%m-%dT%H:%M:%S|%Y-%m-%d %H:%M:%S]. Unix timestamps as seconds or milliseconds since the Unix epoch are now also supported.
• Started moving towards a TyperParameter, not quite finished yet with some ugly imports from within _click but we'll deal with those in follow-up work

Tests with same behaviour on master

• Added test_bool_convert_valid to ensure that the "bool" conversion has remained the same for all kinds of inputs.
• Rewrote several tests to use Typer public API instead of internals
• test_path_resolves, cf point 1) in the Fable review 👇

Breaking changes

(more or less in order of breakingness from most to least)

• Removed support for click_type and open bounds through min_open and max_open. This greatly simplifies the code base while still providing an alternative by setting parser instead.
• A list of choices is now shown as <list[Eggs|Bacon|Cheese]> (before there was no visual distinction between a single choice, or a list of them).
• When not providing a default for DateTime, we just show "<datetime>". Before, it would show the (only) 3 options, but those are not exhaustive anymore. When the user provides actual formats, those are shown (as before).
• Validation error messages have changed from Click's to Pydantic's phrasing. This mostly requires updating test suites and otherwise shouldn't impact users too badly.
• Removed repr functionality of param.type. This wasn't really used except for the tests that were recently added in ➖ Vendor Click and streamline Typer's functionality and code base #1774.
• A parameter typed as Any does NOT raise RuntimeError: Type not yet supported anymore, but instead just falls through and gets a generic TypeAdapter(annotation)

Note that as before, nothing in the module typer/_click (a remnant of the recent vendoring) should be used directly by users and all of it is subject to change in the near future. Any changes in those modules are not considered to be "breaking".

Bug fixes (also breaking bwd compat)

• def main(age: int = typer.Option(15.3)) will now throw a validation error by Pydantic instead of int(15.3) converting it to 15. I consider this a bug fix instead of a regression, and have added a test test_int_rejects_float_default for it.
• Tuples with types that aren't allowed to be None (e.g. tuple[str, int, bool]), will fail validation as soon as one of those elements is None. Our documented example of setting that tuple's default to (None, None, None) is not valid anymore, it should be set to None instead. cf. change to ‎docs_src/multiple_values/options_with_multiple_values/tutorial001_an_py310.py.
• Types are now inferred from the default if the parameter is untyped. Fixes Default value of `False` cast to `'False'` and is thus True #942, cf. new test test_default_infers_param_type.
• With Rich disabled, types were always shown for options, but for arguments only when they were "informative" like a tuple or datetime. This PR makes everything internally consistent, and similar to how it looks with Rich enabled, by always showing the types also for arguments.

Decision points

• _parse_cli_bool was created to ensure we still parse "" as False and to strip whitespace from something like " True ". This is just to mimic old Click behaviour. If we don't do this preprocessing (cleaner code base), the empty string and the non-stripped strings won't pass Pydantic validation and it would be slightly breaking.
• How to display the datetime type if no formats are provided by the user (cf ☝️ "Breaking changes")
• Should we allow an int or float as datetime input, to present Unix timestamps as seconds or milliseconds since the Unix epoch? Cf also Fable's concern in point 4 of its review below 👇
• Should we try to do less with Pydantic to avoid runtime increase? (cf point 7 in Fable's review)

AI Disclaimer

Cursor was used as a micro-managed junior. Every edit was reviewed & understood by me.

TODO

• Mention Pydantic dependency in the docs
• Update the tutorial, focus on (changed) error messages
• Fix coverage
• Merge in the fix from 🏷️ Make Parameter.name typed as str instead of str | None #1878
• I added tests for the change.
• Coverage stays at 100%.
• Metavar printing that came up in this PR has already been fixed in tests & documentation in 💥 Update metavar printing #1863.
• Self review with Fable, after which: "This v2 is a substantial improvement — most of what we discussed landed, and several of the fixes are better than what I suggested." 😎

Follow-up work

• Continue cleaning up code in typer/_click
• Expand code/docs/tests to support many more data types (should be straightforward?!)
• Test/document Pydantic models for re-usable CLI Options

[svlandeg]

This test currently mimics master behaviour 100%.

[github-actions]

📝 Docs preview

Last commit 7e0e6d3 at: https://e0c046eb.typertiangolo.pages.dev

Modified Pages

• https://e0c046eb.typertiangolo.pages.dev/ - (before)
• https://e0c046eb.typertiangolo.pages.dev/tutorial/arguments/default/ - (before)
• https://e0c046eb.typertiangolo.pages.dev/tutorial/arguments/envvar/ - (before)
• https://e0c046eb.typertiangolo.pages.dev/tutorial/arguments/help/ - (before)
• https://e0c046eb.typertiangolo.pages.dev/tutorial/arguments/optional/ - (before)
• https://e0c046eb.typertiangolo.pages.dev/tutorial/commands/arguments/ - (before)
• https://e0c046eb.typertiangolo.pages.dev/tutorial/commands/help/ - (before)
• https://e0c046eb.typertiangolo.pages.dev/tutorial/install/ - (before)
• https://e0c046eb.typertiangolo.pages.dev/tutorial/multiple-values/arguments-with-multiple-values/ - (before)
• https://e0c046eb.typertiangolo.pages.dev/tutorial/multiple-values/options-with-multiple-values/ - (before)
• https://e0c046eb.typertiangolo.pages.dev/tutorial/options/help/ - (before)
• https://e0c046eb.typertiangolo.pages.dev/tutorial/options/required/ - (before)
• https://e0c046eb.typertiangolo.pages.dev/tutorial/parameter-types/ - (before)
• https://e0c046eb.typertiangolo.pages.dev/tutorial/parameter-types/datetime/ - (before)
• https://e0c046eb.typertiangolo.pages.dev/tutorial/parameter-types/enum/ - (before)
• https://e0c046eb.typertiangolo.pages.dev/tutorial/parameter-types/number/ - (before)
• https://e0c046eb.typertiangolo.pages.dev/tutorial/parameter-types/path/ - (before)
• https://e0c046eb.typertiangolo.pages.dev/tutorial/parameter-types/uuid/ - (before)
• https://e0c046eb.typertiangolo.pages.dev/tutorial/typer-app/ - (before)

[svlandeg]

Feedback inline 👇

Boolean options reject "true"/"false" from environment variables.

Yep, good catch. The bool parsing is one of the last things I changed because of the comment in Fable's review about start up cost and it broke the envvar parsing. I pushed a few commits to address this (actually simplifying the bool parsing as a whole) and extended the test suite accordingly, with a variant of the proposed test_bool_option_from_env.

Required list options accept empty configuration values and pass None to the command.

Yep, this was another regression. Fixed by checking for the empty list specifically when determining value_is_missing(). Additional cases are folded into test_required_list_option_missing, covering the same regression as the proposed test_required_list_rejects_empty_configuration would have done.

tuple[FileText, int] leaves the integer element as a string, including invalid input. This is lower priority given the planned removal of file parameters, we might just accept it for now.

Ah, yep. This was tested but with a str which gave the false idea that this was actually working correctly. Updated the relevant test to use int and fixed the actual regression. Also, implemented an additional safe guard that prevents having opened files around when another non-file argument in the tuple crashes upon coercion.

Conclusion

These were 3 actual regressions from master to this branch. I executed the proposed regression tests on the updated version of this branch and they now all succeed. They haven't been included in the test suite ad verbatim though, as I preferred to update/reuse tests where relevant. Should come down to the same thing in the end though.

I'm considering these 3 regressions fixed now.

[tiangolo]

I just remembered another thing I wanted and forgot in the last review.

I would like to make anything new "private" by default.

E.g. rename adapters.py to _adapters.py and so on for new modules/files.

And for things added to existing modules (as we shouldn't change the name of those existing modules/files), then rename the symbols, e.g. describe_number_range to _describe_number_range.

---

Later I want to also make most existing things "private" as well unless intentionally exposed. To prevent people from using internals in ways that we could break in future refactors.

And although we are not yet changing existing things to private, we can still make new things private (unless we intentionally need to expose and document them).

[tiangolo]

I finally managed to finish reviewing all the logic flows and checking all the code and changes. 🚀

It all makes sense to me (as much as I can dig, it's a really big refactor 😅).

I think you went through all of it with enough effort, iterations, and time. I also gave it a review as deep as possible.

I think the sooner we release, the sooner we'll get external feedback. Maybe see what strange corner cases people might have that we didn't predict (and if we want to support them or not).

I'm ready to merge it and release it. 😎

I held myself from any more unnecessary nitpicks so we can release this version already known to work. With that out there we can continue building on top and tweaking things as necessary.

The only thing is making the new things private (unless it doesn't make sense in some cases), but other than that, let me know and I'll click the green button. ✔️

[svlandeg]

@tiangolo:

I updated this branch with master's latest, have a quick look at 1b1ceb7 which changes a control char test case to mimic Pydantic's error formatting, I think this is fine?

And then:

I would like to make anything new "private" by default.

Modules renamed:

• adapters.py → _adapters.py
• coercion.py → _coercion.py
• display.py → _display.py
• param_types.py → _param_types.py

New symbols in existing modules:

• TyperParameter → _TyperParameter (in core.py)
• get_param → _get_param (in main.py)
• get_option_flag_name → _get_option_flag_name (in main.py)
• get_params_ctx_param_name_from_function → _get_params_name_from_function (in main.py)
• get_number_range_help_str → _get_number_range_help_str (TyperArgument function in core.py)
• rich_display_name → _rich_display_name (TyperArgument function in core.py)
• usage_display_name → _usage_display_name (TyperArgument function in core.py)