Skip to content

Cli subcommand with annotation #579

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Cli subcommand with annotation #579

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
43cc284
Initial commit.
kschwab Apr 4, 2025
907a132
Tests.
kschwab Apr 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions pydantic_settings/sources/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ def _annotation_is_complex(annotation: type[Any] | None, metadata: list[Any]) ->

origin = get_origin(annotation)

# Check if annotation is of the form Union[type, ...].
if typing_objects.is_union(origin):
return _union_is_complex(annotation, metadata)
Copy link
Contributor Author

@kschwab kschwab Apr 7, 2025

Choose a reason for hiding this comment

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

@hramezani From what I observed, it appears to be related to incorrectly determining that nested annotated unions are not complex. This change fixed it, but causes other tests fail. I'm not sure if it is the correct fix. Can you take a look?

Copy link
Member

@hramezani hramezani Apr 11, 2025

Choose a reason for hiding this comment

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

Sorry @kschwab for the delay.

I am also not sure what the correct fix is here. As the change breaks some existing tests, I would prefer to remove it.

Copy link
Contributor Author

@kschwab kschwab Apr 12, 2025

Choose a reason for hiding this comment

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

No worries @hramezani. I'm ok with closing this as a won't fix as well.


# Check if annotation is of the form Annotated[type, metadata].
if typing_objects.is_annotated(origin):
# Return result of recursive call on inner type.
Expand Down
15 changes: 15 additions & 0 deletions tests/test_settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,21 @@ class AnnotatedComplexSettings(BaseSettings):
]


def test_cli_nested_annotated_unions(env):
class Cat(BaseModel):
meow: str

class Dog(BaseModel):
woof: str

class Settings(BaseSettings):
model_config = SettingsConfigDict(env_nested_delimiter='__')
animals: Annotated[Union[Annotated[Union[Cat, Dog], 'my_nested_annotation'], None], 'my_annotation']
Copy link
Contributor Author

@kschwab kschwab Apr 7, 2025

Choose a reason for hiding this comment

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

@hramezani I have the top level fix for the CLI in place. However, I discovered that the env parsing does not handle nested annotated unions properly. I've added a test case here to demonstrate the issue.

Copy link
Member

@hramezani hramezani Apr 11, 2025

Choose a reason for hiding this comment

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

this is a special case. I am sure that there are more cases that fail(we are not aware of them).
I would prefer keeping the code base simple and preventing breaking change instead of supporting all complex scenarios.


env.set('ANIMALS__MEOW', 'hiss')
assert Settings().model_dump() == {'animals': {'meow': 'hiss'}}


def test_set_dict_model(env):
env.set('bananas', '[1, 2, 3, 3]')
env.set('CARROTS', '{"a": null, "b": 4}')
Expand Down
4 changes: 2 additions & 2 deletions tests/test_source_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1285,8 +1285,8 @@ class Dog(BaseModel):
class Settings(BaseSettings):
animals: CliSubCommand[Annotated[Union[Cat, Dog], 'my_annotation']]

assert CliApp.run(Settings, cli_args=['Cat', '--meow=purr']).model_dump == {'animals': {'meow': 'purr'}}
assert CliApp.run(Settings, cli_args=['Dog', '--woof=bark']).model_dump == {'animals': {'woof': 'bark'}}
assert CliApp.run(Settings, cli_args=['Cat', '--meow=purr']).model_dump() == {'animals': {'meow': 'purr'}}
assert CliApp.run(Settings, cli_args=['Dog', '--woof=bark']).model_dump() == {'animals': {'woof': 'bark'}}


def test_cli_union_similar_sub_models():
Expand Down
Loading