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

allow Config.field to update a Field #2461

Merged
merged 5 commits into from Mar 3, 2021
Merged

Conversation

samuelcolvin
Copy link
Member

@samuelcolvin samuelcolvin commented Mar 3, 2021

Change Summary

Allow element of Config.field to update elements of a Field

Example of usage:

Related issue number

fix #2426

supersedes #2437

Questions

  • could this break any assumed existing behaviour?
  • do any docs need to be updated? If so, what?

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

@samuelcolvin samuelcolvin added this to the v1.8.1 milestone Mar 3, 2021
@codecov
Copy link

codecov bot commented Mar 3, 2021

Codecov Report

Merging #2461 (9826145) into master (3f84d14) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master     #2461   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           25        25           
  Lines         5100      5109    +9     
  Branches      1048      1050    +2     
=========================================
+ Hits          5100      5109    +9     
Impacted Files Coverage Δ
pydantic/main.py 100.00% <ø> (ø)
pydantic/fields.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3f84d14...9826145. Read the comment docs.

@samuelcolvin samuelcolvin marked this pull request as ready for review March 3, 2021 16:24
Copy link
Member

@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.

LGTM otherwise! Just a question on constraints attributes that don't have None as default value

# attr_name is not an attribute of FieldInfo, it should therefore be added to extra
self.extra[attr_name] = value
else:
if current_value is None:
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 current_value is None:
if current_value is self.__field_constraints__.get(attr_name, None):

I wonder if we should do this to support

from pydantic import BaseModel, Field


class Foo(BaseModel):
    a: str = Field(...)

    class Config:
        validate_assignment = True
        fields = {'a': {'allow_mutation': False}}

f = Foo(a='x')
f.a = 'y'  # should fail

Copy link
Member

Choose a reason for hiding this comment

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

LOL 😄 just saw your last commit

Copy link
Member Author

Choose a reason for hiding this comment

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

ha, I was just writing a comment about allow_mutation.

@samuelcolvin
Copy link
Member Author

For me this is now ready, I'll merge once passing.

@PrettyWood are you okay for me to release v1.8.1 after this? I think we have fixed everything that has been reported.

@PrettyWood
Copy link
Member

@samuelcolvin Yes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dataclass config fields not warranted in json schema (1.8)
2 participants