Fix nested all include exclude #1588
Conversation
xspirus
force-pushed
the
fix-nested-all-include-exclude
branch
from
5bfbf42
to
067f6c7
Compare
Codecov Report
@@ Coverage Diff @@
## master #1588 +/- ##
=======================================
Coverage 99.94% 99.94%
=======================================
Files 21 21
Lines 3863 3895 +32
Branches 767 784 +17
=======================================
+ Hits 3861 3893 +32
Misses 1 1
Partials 1 1
Continue to review full report at Codecov.
|
xspirus
force-pushed
the
fix-nested-all-include-exclude
branch
3 times, most recently
from
cce334a
to
b9c83e1
Compare
changes/1579-xspirus.md
Outdated
| @@ -0,0 +1,2 @@ | |||
| Fix behavior of `__all__` key when used in conjunction with index keys in advanced include/exclude of fields that are | |||
pydantic/utils.py
Outdated
| all_set = items.get('__all__') | ||
| if all_set: | ||
| if all_items: | ||
| default: Type[Union[set, dict]] # type: ignore |
There was a problem hiding this comment.
| default: Type[Union[set, dict]] # type: ignore | |
| default: Type[Union[Set[Any, Dict[Any, Any]] |
Maybe?
pydantic/utils.py
Outdated
| elif isinstance(all_items, AbstractSet): | ||
| default = set | ||
| else: | ||
| default = ... |
There was a problem hiding this comment.
I'm not clear about the significance of the ellipsis here?
tests/test_edge_cases.py
Outdated
|
|
||
| m = Model( | ||
| subs=[ | ||
| SubModel(k=1, subsubs=[SubSubModel(i=1, j=1), SubSubModel(i=2, j=2)]), |
There was a problem hiding this comment.
I think it's clear and more consistent with other tests if you use dicts, not models when creating dicst,
| SubModel(k=1, subsubs=[SubSubModel(i=1, j=1), SubSubModel(i=2, j=2)]), | |
| dict(k=1, subsubs=[dict(i=1, j=1), dict(i=2, j=2)]), |
etc.
tests/test_edge_cases.py
Outdated
| 'subs': [{'k': 1, 'subsubs': []}, {'k': 2, 'subsubs': []}] | ||
| } | ||
| # Merge sub dict-set | ||
| assert m.dict(exclude={'subs': {'__all__': {'subsubs': {0: {'i'}}}, 0: {'subsubs': {1}}}}) == { |
There was a problem hiding this comment.
maybe you could parameterise these?
tests/test_edge_cases.py
Outdated
| ) | ||
|
|
||
| # Normal nested __all__ | ||
| assert m.dict(include={'subs': {'__all__': {'subsubs': {'__all__': {'i'}}}}}) == { |
There was a problem hiding this comment.
and again, I think this should be parameterised.
xspirus
force-pushed
the
fix-nested-all-include-exclude
branch
from
3dcf327
to
d784445
Compare
|
@samuelcolvin Thanks for the review. I hope the docstring makes it a bit clearer on what is going on. Updated the tests as well. |
This commit also fixes some weird cases in the recursive `update_normalized_all` call and Ellipsis values.
xspirus
force-pushed
the
fix-nested-all-include-exclude
branch
from
d784445
to
9b0e8bb
Compare
Change Summary
Fix the behavior of
__all__key, when used in conjunction with index keys in advanced exclude/include of fields that are sequences.Related issue number
Fixes #1579
Checklist
changes/<pull request or issue id>-<github username>.mdfile added describing change(see changes/README.md for details)