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

โœจ Support for pydantic options #803

Draft
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

pypae
Copy link

@pypae pypae commented Apr 24, 2024

๐Ÿ“ Summary

This PR adds support for pydantic options as requested in the following issue: #111
In particular it implements the behavior I described in #111 (comment).

The implementation changes the signature of the callback to add all fields of the (possibly nested) pydantic models of the original callback as typer.Options. As a result the implementation is largely independent of typer and could also be used as a standalone package which could be activated with an additional decorator on the command functions:

Usage as a decorator (draft)
import pydantic
import typer
from typer_pydantic import unwrap_pydantic_models


class User(pydantic.BaseModel):
    id: int
    name: str = "Jane Doe"

@unwrap_pydantic_models
def main(num: int, user: User):
    print(num, type(num))
    print(user, type(user))


if __name__ == "__main__":
    typer.run(main)

๐Ÿ”€ Related PRs

โ˜‘๏ธ To Do

  • Make linter happy
  • Describe advantages over other approaches
  • Add support for typer.Option and typer.Argument inside pydantic models
  • Add tests for nested pydantic models
  • Make sure validation errors are reported properly
  • Make pydantic dependency optional
  • Allow sequences of pydantic models. I suggest implementing indexed list options first, see the comment below.

Copy link

๐Ÿ“ Docs preview for commit bfa4e6d at: https://c0ad6c82.typertiangolo.pages.dev

Copy link

๐Ÿ“ Docs preview for commit a6a7004 at: https://efd73f25.typertiangolo.pages.dev

Copy link

๐Ÿ“ Docs preview for commit 3e37bb5 at: https://f4caeab2.typertiangolo.pages.dev

Copy link

๐Ÿ“ Docs preview for commit 6c76ab9 at: https://cc7a8711.typertiangolo.pages.dev

@svlandeg svlandeg added feature New feature, enhancement or request p3 labels Apr 25, 2024
Copy link

๐Ÿ“ Docs preview for commit ebb5877 at: https://6cfbeacd.typertiangolo.pages.dev

Copy link

๐Ÿ“ Docs preview for commit 12be77e at: https://ab20f60a.typertiangolo.pages.dev

@pypae pypae changed the title Support for pydantic options [Draft] โœจ Support for pydantic options Apr 25, 2024
@pypae
Copy link
Author

pypae commented Apr 26, 2024

Problem

If we want to allow sequences of pydantic models, things get unreadable quite fast.

Let's look at an example of a nested pydantic model with a sequence of another model:
pets.py

from typing import Optional, List

import typer

import pydantic


class Pet(pydantic.BaseModel):
    name: str
    species: str


class Person(pydantic.BaseModel):
    name: str
    age: Optional[float] = None
    pets: List[Pet]


def main(person: Person):
    print(person, type(person))


if __name__ == "__main__":
    typer.run(main)

The script could be called like this:

$ python pets.py --person.name Jeff --person.pets.name Lassie --person.pets.species dog

If we want to add multiple pets, we can just supply --person.pets.name and person.pets.species multiple times.

$ python pets.py --person.name Jeff --person.pets.name Lassie --person.pets.species dog --person.pets.name Nala --person.pets.species cat

We don't explicitly state which pet names and species belong together and have to rely on the correct order of parameters. In my opinion this is potentially confusing for the CLI user and may lead to bugs.

Potential Solution

To make the mapping more explicit, we could allow to enable typer.Option lists to be indexed.
Like for nested pydantic models, I suggest sticking to the syntax traefik uses for lists. I.e. entrypoints.<name>.http.tls.domains[n].main

Indexed lists could be implemented independently of this PR and should work for all lists.
I suggest adding an indexed flag on typer.Option like shown in the example below.

indexed_list.py

from typing import List

import typer

def main(indexed_list: List[int] = typer.Option(..., indexed=True)):
    print(indexed_list)

if __name__ == "__main__":
    typer.run(main)

This would then produce the following help text:

$ python indexed_list.py --help
                                                                                
 Usage: indexed_list.py [OPTIONS]                                    
                                                                    
โ•ญโ”€ Options โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ฎ
โ”‚ *  --indexed-list[n]        INTEGER   [default: None] [required] โ”‚
โ”‚    --help                             Show this message and      โ”‚
โ”‚                                       exit.                      โ”‚
โ•ฐโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ฏ

And could be used like this:

$ python indexed_list.py --indexed-list[1] 0 --indexed-list[0] 1 --indexed-list[2] 2
[1, 0, 2]

Note how the order of the input parameters doesn't matter anymore because the indices are given explicitly.

Notes on Implementation

Implementing this might not be trivial, but I think it could be possible by forwarding unknown options as described in the click docs.

Edit: This might actually be easier using token normalization.


Do you have any opinions on this?

@itepifanio
Copy link

@pypae why limiting the pydantic models to typer.Option? It would be nice to be able to define arguments in the pydantic model as well, for example:

class User(BaseModel):
        name: Annotated[str, typer.Argument()]      
        lastname: Annotated[str, typer.Option()]            

    @app.command()
    def cmd(user: User):
        pass

My current use case requires a set of commands with a client argument, so this would allow code reuse.

If we want to allow sequences of pydantic models, things get unreadable quite fast [...] In my opinion this is potentially confusing for the CLI user and may lead to bugs

I'm not sure how many users would required deep nested commands in their CLI, and if that's the case using a toml or yaml config file may be a better way to do that. Adding a warning note at the doc should be enough for this feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature, enhancement or request p3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants