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

Fixes discriminated unions not working on aliased literal fields. Issue #3849. #5736

Merged

Conversation

benwah
Copy link

@benwah benwah commented May 10, 2023

Change Summary

The issue is described here: #3849

TL;DR:

When you have an aliased field, and it uses a discriminated union; you can no longer use the un-aliased field name to instantiate the model.

According to the discussion, this is not going to be a problem once Pydantic 2.x releases, but it's definitly problematic for 1.10.x which I'm currently using.

Here's the test failure without this change:

============================================================== FAILURES ==============================================================
_________________________________________ test_discriminated_union_validation_aliased_field __________________________________________

    def test_discriminated_union_validation_aliased_field():
        def to_upper(field_name: str) -> str:
            return field_name.upper()
    
    
        class PyBase(BaseModel):
            class Config:
                allow_population_by_field_name = True
                alias_generator = to_upper
    
    
        class Dog(PyBase):
            pet_type: Literal["dog"]
            first_name: str
    
    
        class Cat(PyBase):
            pet_type: Literal["cat"]
            first_name: str
    
    
        Pet = Annotated[Cat | Dog, Field(discriminator="pet_type")]
    
    
        class Model(PyBase):
            pet: Pet
    
    
>       m = Model.parse_obj({'pet': {'pet_type': 'cat', "first_name": "Benoit"}})

tests/test_discrimated_union.py:116: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
pydantic/main.py:526: in parse_obj
    return cls(**obj)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

__pydantic_self__ = Model(), data = {'pet': {'first_name': 'Benoit', 'pet_type': 'cat'}}, values = {}, fields_set = {'pet'}
validation_error = ValidationError(model='Model', errors=[{'loc': ('PET',), 'msg': "Discriminator 'pet_type' is missing in value", 'type': 'value_error.discriminated_union.missing_discriminator', 'ctx': {'discriminator_key': 'pet_type'}}])

    def __init__(__pydantic_self__, **data: Any) -> None:
        """
        Create a new model by parsing and validating input data from keyword arguments.
    
        Raises ValidationError if the input data cannot be parsed to form a valid model.
        """
        # Uses something other than `self` the first arg to allow "self" as a settable attribute
        values, fields_set, validation_error = validate_model(__pydantic_self__.__class__, data)
        if validation_error:
>           raise validation_error
E           pydantic.error_wrappers.ValidationError: 1 validation error for Model
E           PET
E             Discriminator 'pet_type' is missing in value (type=value_error.discriminated_union.missing_discriminator; discriminator_key=pet_type)

pydantic/main.py:341: ValidationError
====================================================== short test summary info =======================================================
FAILED tests/test_discrimated_union.py::test_discriminated_union_validation_aliased_field - pydantic.error_wrappers.ValidationError...
========================================================= 1 failed in 0.17s ==========================================================

Related issue number

Fix #3849

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

Selected Reviewer: @PrettyWood

@benwah benwah force-pushed the fix_discriminated_union_issue_3849 branch from ebc5266 to a797d73 Compare May 10, 2023 16:00
@benwah
Copy link
Author

benwah commented May 10, 2023

please review

@benwah benwah force-pushed the fix_discriminated_union_issue_3849 branch 3 times, most recently from 1d26e81 to fcf0bdf Compare May 10, 2023 16:14
@benwah
Copy link
Author

benwah commented May 10, 2023

Hypothesis test suite is failing in my local tests, with and without these changes. So I don't think this is a regression.

image

@benwah benwah force-pushed the fix_discriminated_union_issue_3849 branch from fcf0bdf to 6173cfa Compare May 10, 2023 16:29
pydantic/fields.py Outdated Show resolved Hide resolved
@benwah benwah force-pushed the fix_discriminated_union_issue_3849 branch from 6173cfa to 1402ac0 Compare May 10, 2023 23:49
@benwah benwah force-pushed the fix_discriminated_union_issue_3849 branch from 1402ac0 to 4d2a64c Compare May 10, 2023 23:55
@dmontagu dmontagu assigned dmontagu and unassigned PrettyWood May 12, 2023
@dmontagu dmontagu merged commit d61792c into pydantic:1.10.X-fixes May 12, 2023
49 checks passed
renovate bot added a commit to allenporter/flux-local that referenced this pull request May 25, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [pydantic](https://togithub.com/pydantic/pydantic) | `==1.10.7` ->
`==1.10.8` |
[![age](https://badges.renovateapi.com/packages/pypi/pydantic/1.10.8/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/pypi/pydantic/1.10.8/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/pypi/pydantic/1.10.8/compatibility-slim/1.10.7)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/pypi/pydantic/1.10.8/confidence-slim/1.10.7)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>pydantic/pydantic</summary>

###
[`v1.10.8`](https://togithub.com/pydantic/pydantic/releases/tag/v1.10.8):
2023-05-23

[Compare
Source](https://togithub.com/pydantic/pydantic/compare/v1.10.7...v1.10.8)

#### What's Changed

- Fix a bug in `Literal` usage with `typing-extension==4.6.0`,
[#&#8203;5826](https://togithub.com/pydantic/pydantic/issues/5826) by
[@&#8203;hramezani](https://togithub.com/hramezani)
- This solves the (closed) issue
[#&#8203;3849](https://togithub.com/pydantic/pydantic/issues/3849) where
aliased fields that use discriminated union fail to validate when the
data contains the non-aliased field name,
[#&#8203;5736](https://togithub.com/pydantic/pydantic/issues/5736) by
[@&#8203;benwah](https://togithub.com/benwah)
- Update email-validator dependency to >=2.0.0post2,
[#&#8203;5627](https://togithub.com/pydantic/pydantic/issues/5627) by
[@&#8203;adriangb](https://togithub.com/adriangb)
- update `AnyClassMethod` for changes in
[python/typeshed#&#8203;9771](https://togithub.com/python/typeshed/issues/9771),
[#&#8203;5505](https://togithub.com/pydantic/pydantic/issues/5505) by
[@&#8203;ITProKyle](https://togithub.com/ITProKyle)

#### New Contributors

-
[@&#8203;DimitriPapadopoulos](https://togithub.com/DimitriPapadopoulos)
made their first contribution in
[pydantic/pydantic#5375
- [@&#8203;ITProKyle](https://togithub.com/ITProKyle) made their first
contribution in
[pydantic/pydantic#5505
- [@&#8203;benwah](https://togithub.com/benwah) made their first
contribution in
[pydantic/pydantic#5736

**Full Changelog**:
pydantic/pydantic@v1.10.7...v1.10.8

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/allenporter/flux-local).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS45OC40IiwidXBkYXRlZEluVmVyIjoiMzUuOTguNCIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
renovate bot added a commit to allenporter/pyrainbird that referenced this pull request May 25, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [pydantic](https://togithub.com/pydantic/pydantic) | `==1.10.7` ->
`==1.10.8` |
[![age](https://badges.renovateapi.com/packages/pypi/pydantic/1.10.8/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/pypi/pydantic/1.10.8/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/pypi/pydantic/1.10.8/compatibility-slim/1.10.7)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/pypi/pydantic/1.10.8/confidence-slim/1.10.7)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>pydantic/pydantic</summary>

###
[`v1.10.8`](https://togithub.com/pydantic/pydantic/releases/tag/v1.10.8):
2023-05-23

[Compare
Source](https://togithub.com/pydantic/pydantic/compare/v1.10.7...v1.10.8)

##### What's Changed

- Fix a bug in `Literal` usage with `typing-extension==4.6.0`,
[#&#8203;5826](https://togithub.com/pydantic/pydantic/issues/5826) by
[@&#8203;hramezani](https://togithub.com/hramezani)
- This solves the (closed) issue
[#&#8203;3849](https://togithub.com/pydantic/pydantic/issues/3849) where
aliased fields that use discriminated union fail to validate when the
data contains the non-aliased field name,
[#&#8203;5736](https://togithub.com/pydantic/pydantic/issues/5736) by
[@&#8203;benwah](https://togithub.com/benwah)
- Update email-validator dependency to >=2.0.0post2,
[#&#8203;5627](https://togithub.com/pydantic/pydantic/issues/5627) by
[@&#8203;adriangb](https://togithub.com/adriangb)
- update `AnyClassMethod` for changes in
[python/typeshed#&#8203;9771](https://togithub.com/python/typeshed/issues/9771),
[#&#8203;5505](https://togithub.com/pydantic/pydantic/issues/5505) by
[@&#8203;ITProKyle](https://togithub.com/ITProKyle)

##### New Contributors

-
[@&#8203;DimitriPapadopoulos](https://togithub.com/DimitriPapadopoulos)
made their first contribution in
[pydantic/pydantic#5375
- [@&#8203;ITProKyle](https://togithub.com/ITProKyle) made their first
contribution in
[pydantic/pydantic#5505
- [@&#8203;benwah](https://togithub.com/benwah) made their first
contribution in
[pydantic/pydantic#5736

**Full Changelog**:
pydantic/pydantic@v1.10.7...v1.10.8

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/allenporter/pyrainbird).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS45OC40IiwidXBkYXRlZEluVmVyIjoiMzUuOTguNCIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants