Skip to content

Conversation

@MAD-py
Copy link
Contributor

@MAD-py MAD-py commented Jul 25, 2021

Change Summary

In the validate_arguments decorator the Field object can be used to provide additional information.

@validate_arguments
def how_many(num: Annotated[int, Field(gt=10)]):
    return num

Now the Field alias parameter will also work in the decorator, the idea is to be able to add this functionality that exists in the creation of the models and did not work so far in the decorator.

@validate_arguments
def foo(bar: Annotated[str, Field(alias="quux")]):
    print(bar)

foo(quux=42)

Related issue number

With this change issue #2429 can be considered closed.

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • changes/<pull request or issue id>-<github username>.md file added describing change
    (see changes/README.md for details)
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

@MAD-py
Copy link
Contributor Author

MAD-py commented Jul 25, 2021

Please review the solution I gave to issue #2429 when you have time.

I remain attentive to any correction or any questions you would have about what I did.

Copy link
Collaborator

@PrettyWood PrettyWood left a comment

Choose a reason for hiding this comment

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

Could you please add at least a second parameter on one of the functions in your test?
Other than that looks good to me for the code 👍 Please update
I'm surprised on the need though. I feel like having an alias for a function is very weird x) mypy will complain and it's hard to read. I though the need would be for constraints on range, regex and stuff only

@PrettyWood PrettyWood added awaiting author revision awaiting changes from the PR author and removed ready for review labels Sep 4, 2021
Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

and as @PrettyWood requested, please can we have more complete/challenging tests.

non_var_fields = set(self.model.__fields__) - {self.v_args_name, self.v_kwargs_name}
for k, v in kwargs.items():
if k in non_var_fields:
if k in non_var_fields or k in fields_alias:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if k in non_var_fields or k in fields_alias:
if k in non_var_fields or k in fields_alias is not None:

see #4252

@samuelcolvin
Copy link
Member

please update

@github-actions github-actions bot assigned MAD-py and unassigned samuelcolvin and PrettyWood Aug 4, 2022
@MAD-py
Copy link
Contributor Author

MAD-py commented Aug 5, 2022

Hi @samuelcolvin.

This would be the update of the tests and the integration of the empty string like a possible alias is not necessary because it is already built in as the tests show.

@samuelcolvin samuelcolvin merged commit 7431683 into pydantic:master Aug 8, 2022
@samuelcolvin
Copy link
Member

thanks so much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting author revision awaiting changes from the PR author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants