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

feat: make dict validator strict by default #2211

Closed
wants to merge 1 commit into from

Conversation

PrettyWood
Copy link
Member

@PrettyWood PrettyWood commented Dec 20, 2020

Change Summary

Make the dict_validator strict by default.
Add a way to keep old behaviour with loose_dict_validator flag in BaseConfig

Related issue number

closes #1268
closes #1982

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)

@codecov
Copy link

codecov bot commented Dec 20, 2020

Codecov Report

Merging #2211 (4f6ad91) into master (3496a47) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master     #2211   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           21        21           
  Lines         4121      4121           
  Branches       829       830    +1     
=========================================
  Hits          4121      4121           
Impacted Files Coverage Δ
pydantic/dataclasses.py 100.00% <100.00%> (ø)
pydantic/error_wrappers.py 100.00% <100.00%> (ø)
pydantic/fields.py 100.00% <100.00%> (ø)
pydantic/validators.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 3496a47...8e18ad1. Read the comment docs.

@samuelcolvin
Copy link
Member

I think a feature flag for this is too confusing. We should just wait for v2, hopefully not too long now.

I'm really hoping to make some time to do the refactoring required for v2 in january.

@samuelcolvin samuelcolvin added the deferred deferred until future release or until something else gets done label Jan 1, 2021
@PrettyWood
Copy link
Member Author

Glad to read that! Don't hesitate if we can help :)
If this is deferred I would like to still add the documentation to explain the "how to change globally pydantic behaviour" probably in #2193 . WDYT?

@samuelcolvin
Copy link
Member

I'm always happy to accept a PR to improve documentation.

I'm just hesitant to make fundamental behaviour changes (even behind feature flags) within v1, especially since I think we're approaching the end of the v1 lifespan.

@HansBrende
Copy link

@PrettyWood @samuelcolvin here are some slightly less intrusive tweaks that would fix the bugs while retaining the existing behavior of being able to parse a dict from a list of tuples... would it make sense to add something like one of these before v2?

#2513 (comment)
#2513 (comment)

I would be happy to submit a PR (if desired), or not, if I hear either an affirmative or a negative response, respectively...

@samuelcolvin
Copy link
Member

fixed in v2, omit for v1.10

@PrettyWood PrettyWood deleted the strict-dict-validator branch August 4, 2022 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deferred deferred until future release or until something else gets done
Projects
None yet
Development

Successfully merging this pull request may close these issues.

__root__ field parsing doesn't depend on Union type order Make dict validation stricter
3 participants