Use ChoiceKey in TaggedUnionValidator for int keys

[philhchen]

Used for supporting Discriminated Unions for pydantic/pydantic#4675

As per pydantic/pydantic#4675 (comment), supporting int and string keys should use an Enum.

[samuelcolvin]

Looking good, please put the issue being fixed in the pr body.

[codspeed-hq]

CodSpeed Performance Report

Merging #405 phil/choice-key (f743b90) will not alter performances.

Summary

🔥 0 improvements
❌ 0 regressions
✅ 85 untouched benchmarks

🆕 0 new benchmarks
⁉️ 0 dropped benchmarks

[samuelcolvin]

looking good, please add tests for the new cases.

[philhchen]

@samuelcolvin I realize this PR doesn't support Python Enum classes yet - I don't think that should be too hard to add into here though.

[samuelcolvin]

it should support enums (please add a test) if they also inherit form str or int (e.g. IntEnum or StrEnum).

Supporting more general enums is probably a bigger issue.

[codecov-commenter]

Codecov Report

Merging #405 (f743b90) into main (3ea00bd) will decrease coverage by 0.02%.
The diff coverage is 98.30%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #405      +/-   ##
==========================================
- Coverage   95.33%   95.31%   -0.02%     
==========================================
  Files          90       90              
  Lines       10536    10564      +28     
  Branches        8        8              
==========================================
+ Hits        10044    10069      +25     
- Misses        492      495       +3     

Impacted Files	Coverage Δ
src/input/input_json.rs	95.88% <ø> (ø)
src/validators/union.rs	98.37% <97.50%> (+0.13%)	⬆️
pydantic_core/core_schema.py	98.79% <100.00%> (+<0.01%)	⬆️
src/errors/location.rs	98.27% <100.00%> (+0.09%)	⬆️
src/validators/literal.rs	97.84% <100.00%> (+<0.01%)	⬆️
src/errors/line_error.rs	92.77% <0.00%> (-3.62%)	⬇️

---

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 3ea00bd...f743b90. Read the comment docs.

[dmontagu]

@philhchen I have been working on integrating this on the pydantic side in pydantic/pydantic#5051; this is great work, very much appreciated!

I've got many discriminated-union tests now passing in the branch from the PR above (with a local install of this branch for pydantic_core), including using int literals as the discriminator. (It doesn't look like it's passing in CI because it needs this PR's changes, but that's okay 😄).

However, I have noticed two weird issues that I'm hoping you (or @samuelcolvin) will see how to resolve faster than me.

Here is the test that I have been working against:

https://github.com/pydantic/pydantic/blob/0aa55db15ffd92343ce4ece22ecb3a0d237bef46/tests/test_discrimated_union.py#L332-L370

(I know the test needs modification for the final bit for the string cases, but it's not getting that far yet)

• In the case of TestStrEnum (which is essentially equivalent to creating a class that inherits from str and Enum), I get the following error while trying to parse the "valid" data:

        @classmethod
        def model_validate(cls: type[Model], obj: Any) -> Model:
    >       values, fields_set = cls.__pydantic_validator__.validate_python(obj)
    E       pydantic_core._pydantic_core.ValidationError: 1 validation error for B
    E       sub -> v_b -> m
    E         Input should be a string, not an instance of a subclass of str [type=string_sub_type, input_value=<EnumValue.b: 'v_b'>, input_type=EnumValue]
  

  It looks like for StrEnum's to work, we need to modify it to allow proper subclasses of str. @samuelcolvin do you know whether this will end up being very involved?

• While the test is passing for IntEnum, you'll notice that the error format it is looking for in the expected-bad input data is actually showing the tag values as though they are strings:

    'ctx': {'discriminator': "'m'", 'expected_tags': "'1', '2'", 'tag': '3'},
  

  It's possible I made a mistake somewhere on the python/pydantic side, but I think this is something to do with how the error message is being generated within pydantic_core.

(Also, I needed to change the default value of from_attributes to True for the TaggedUnionValidator, as @samuelcolvin mentioned above. I'll push a commit to this branch making that change.)

[dmontagu]

I just realized that you had made that change, then undid it in favor of #398 😅; I just closed that PR, and pushed a commit to this PR updating the relevant file and fixing the test.

[philhchen]

While the test is passing for IntEnum, you'll notice that the error format it is looking for in the expected-bad input data is actually showing the tag values as though they are strings:

'ctx': {'discriminator': "'m'", 'expected_tags': "'1', '2'", 'tag': '3'},

Yeah I noticed this as well - I think it's a result of this line. I'll modify it instead of being lazy 😅

[philhchen]

Ok I think int tags should now show up as ints in their representations as error messages

[philhchen]

In the case of TestStrEnum (which is essentially equivalent to creating a class that inherits from str and Enum), I get the following error while trying to parse the "valid" data

I was able to reproduce this and traced down the error to a nuanced bug in these lines.

The logic there is that if a literal can be downcast as a str, then it is treated as a LiteralSingleString, but LiteralSingleString actually uses strict_str to validate inputs. Even if we were to treat these using the LiteralGeneralValidator, that uses the same logic in these lines.

@samuelcolvin do you have some thoughts on how to proceed?

[samuelcolvin]

Would this be solved by using validate_str in LiteralSingleString?

I'll look a bit later when I'm at a computer.

[philhchen]

How do you know if it validate_str should be strict = true or not?

[samuelcolvin]

I guess based on the runtime strict value.

[philhchen]

@dmontagu I don't have write access to the pydantic repository so I have a PR on top of your branch here: pydantic/pydantic#5100

[samuelcolvin]

I've made a few tweaks. I think this is ready.

@dmontagu @philhchen are you happy to merge this?