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.

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

Jump to bottom

Add support for field alias in dataclass signature #8387

Merged
merged 3 commits into from Dec 20, 2023
Merged

Add support for field alias in dataclass signature #8387

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9b73703
Fixing #8367
NeevCohen Dec 16, 2023
ab9ae9f
Change helper function name
NeevCohen Dec 19, 2023
f5de0da
Remove redundant if check
NeevCohen Dec 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 20 additions & 5 deletions pydantic/_internal/_constructor_signature_generators.py
View file
Open in desktop
Expand Up @@ -10,6 +10,21 @@
from ..fields import FieldInfo


def _field_name_or_alias(field_name: str, field_info: FieldInfo) -> str:
"""Extract the correct name to use for the field when generating a signature.
If it has a valid alias then returns its alais, else returns its name
Args:
field_name: The name of the field
field_info: The field

Returns:
The correct name to use when generating a signature.
"""
return (
field_info.alias if isinstance(field_info.alias, str) and is_valid_identifier(field_info.alias) else field_name
)


def generate_pydantic_signature(
init: Callable[..., None],
fields: dict[str, FieldInfo],
Expand Down Expand Up @@ -37,6 +52,9 @@ def generate_pydantic_signature(
for param in islice(present_params, 1, None): # skip self arg
# inspect does "clever" things to show annotations as strings because we have
# `from __future__ import annotations` in main, we don't want that
if fields.get(param.name):
param_name = _field_name_or_alias(param.name, fields[param.name])
param = param.replace(name=param_name)
Copy link
Member

Choose a reason for hiding this comment

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

I don't love mutating the param object here. I think we could find a better solution that's less redundant...

Copy link
Member

Choose a reason for hiding this comment

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

I'll note, the parameter_post_processor function for the dataclass signature generation also has alias extraction logic, but that only applies when we have a dataclass field using pydantic's Field as a default.

Copy link
Member

Choose a reason for hiding this comment

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

Ahh, I see why you've modified the param.name here... perhaps that is fine for now then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't love mutating the param object here. I think we could find a better solution that's less redundant...

Yeah it seemed iffy to me too, but the that's what the existing code did so I preferred to stick it. Do you think I should save it in a different variable?

Copy link
Contributor Author

@NeevCohen NeevCohen Dec 18, 2023 •
edited

Choose a reason for hiding this comment

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

I'll note, the parameter_post_processor function for the dataclass signature generation also has alias extraction logic, but that only applies when we have a dataclass field using pydantic's Field as a default.

Yeah I just looked at process_param_defaults() which gets passed as parameter_post_processor. I thought perhaps I should just add the logic there and avoid the duplication of the alias extraction logic.

Perhaps adding another parameter to it like so

def process_param(param: Parameter, field: Optional[FieldInfo] = None) -> Parameter:
    """Modify the signature for a parameter in a dataclass. If a parameter has a Model field associated with it, use it for modifying the parameter.

    Args:
        param (Parameter): The parameter
        field: An optional Model field associated with the parameter

    Returns:
        Parameter: The custom processed parameter
    """
    # Logic that uses the field if it exists or the parameter default
    ....

Do you think this is good solution?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I like the thought, though overall I'm tempted to not increase the complexity of that function.

Why don't we stick with something similar to your original solution, then do a more significant refactoring to fix #7978 in a separate PR? Could you make the helper function that you've designed private, for now?

As a side note, looks like we changed up the logic for the signature pattern in this PR, which did represent a major reduction in redundancy across signature generation logic: https://github.com/pydantic/pydantic/pull/7925/files.

if param.annotation == 'Any':
param = param.replace(annotation=Any)
if param.kind is param.VAR_KEYWORD:
Expand All @@ -48,16 +66,13 @@ def generate_pydantic_signature(
allow_names = config_wrapper.populate_by_name
for field_name, field in fields.items():
# when alias is a str it should be used for signature generation
if isinstance(field.alias, str):
param_name = field.alias
else:
param_name = field_name
param_name = _field_name_or_alias(field_name, field)

if field_name in merged_params or param_name in merged_params:
continue

if not is_valid_identifier(param_name):
if allow_names and is_valid_identifier(field_name):
if allow_names:
param_name = field_name
else:
use_var_kw = True
Expand Down
3 changes: 2 additions & 1 deletion tests/test_dataclasses.py
View file
Open in desktop
Expand Up @@ -2487,9 +2487,10 @@ class Model:
a: float = dataclasses.field(default_factory=float)
b: float = Field(default=1.0)
c: float = Field(default_factory=float)
d: int = dataclasses.field(metadata={'alias': 'dd'}, default=1)

assert str(inspect.signature(Model)) == (
"(x: int, y: str = 'y', z: float = 1.0, a: float = <factory>, b: float = 1.0, c: float = <factory>) -> None"
"(x: int, y: str = 'y', z: float = 1.0, a: float = <factory>, b: float = 1.0, c: float = <factory>, dd: int = 1) -> None"
)


Expand Down