BUG: Order matters in Union type hints?

[tangkong]

Expected Behavior

Order shouldn't matter for Union type hints (eg Union[int, float] vs Union[float, int]

Current Behavior

I'll be a bit verbose here as I'm investigating this a bit myself

Say you have a plan that takes an argument with typing Union[float, int]:

def my_plan(dets: List[Any], num: typing.Union[int, float], default_arg=1):
    yield from count(dets, num=num)

one would expect that both these items would verify successfully:

{item = {"name": "my_plan", "args": [["det"], 1.2], "user_group": "root"}
{item = {"name": "my_plan", "args": [["det"], 1], "user_group": "root"}

My quick testing reveals:

	Union[int, float]	Union[float, int]
num = 1	Pass	Pass
num = 1.2	Fail	Pass

It appears that when creating the pydantic model, pydantic attempts to cast the value to the first type in the Union, creating a mismatch between the value in the model and the provided value

Details

> /Users/roberttk/src/bluesky-queueserver/bluesky_queueserver/manager/profile_ops.py(2427)_validate_plan_parameters()
-> m = pydantic_validate_model(bound_args.arguments, pydantic_model_class)
(Pdb) m = pydantic_model_class(**bound_args.arguments)
(Pdb) m
Model(dets=['det'], num=10, default_arg=1)
(Pdb) bound_args.arguments
{'dets': ['det'], 'num': 10.5}

This could cause issues if we have something like Union[str, int] as well, as pydantic would always cast your input to a string and fail to match the input.

Possible Solution

Always put float before int? be reaaaaally clear in documentation? Is this something that pydantic v2 addresses?

Context

annotating plans and having them not validate

Your Environment

bluesky_queueserver=0.0.18, though I don't think this was addressed in 0.0.19?

[klauer]

Can smart union help resolve this? https://docs.pydantic.dev/1.10/usage/model_config/#smart-union

[dmgav]

I am out of office until the end of the week and have no access to computer. I wonder if it behaves the same with v0.0.19 and Pydantic v2, which performs strict type checking by default. Also, what is the error message?

[tangkong]

Upgrading to pydantic v2 does seem to alleviate the issue.

There was no error message per-se, rather validation errors.

plan_validated: False, Plan validation failed: Error in argument types: Incorrect parameter type: key='num', value='10.5' (<class 'float'>), interpreted as '10' (<class 'int'>)

[dmgav]

The error message is generated by this code:

bluesky-queueserver/bluesky_queueserver/manager/profile_ops.py

Lines 2426 to 2428 in a6d9b48

	success, msg = _compare_in_out(bound_args.arguments, m.__dict__)
	if not success:
	raise ValueError(f"Error in argument types: {msg}")

The problem with Pydantic 1 is that it is focused on data conversion and does not do strict type checking. As long as data can be converted to the target type, Pydantic 1 is happy. For example, if a parameter type is int, the inputs 10.3 (float is converted to 10 by discarding .3), "10" (string is converted to integer), "10.3" (string is converted to integer and fractional part is discarded) are all accepted by Pydantic 1 and this is not the desired behavior in our case. The _compare_in_out() function compares the types of input and output values and raises the exception if type was converted by Pydantic (error message 'Incorrect parameter type'. Pydantic 2 performs strict type checking by default and raises exceptions for inputs of incorrect type are provided (I think it may still convert float 10.0 to int 10, but it will not discard fractional part). Therefore, the code referenced above is not necessary and should not be called if the server is run with Pydantic 2.

It may be possible to improve the code by implementing strict type checking with Pydantic 1 (using custom validators?), but I am not sure it is worth working on it considering that this is the default behavior of Pydantic 2 .

[tangkong]

I'd advocate for at least pinning pydantic > 2 if you choose not to address this for pydantic 1. If an environment somehow solves for pydantic 1 (as ours does currently), this behavior would still be present and very problematic.

[dmgav]

I think pinning is a good idea, but I would wait until other packages can work with Pydantic 2. I am not sure Tiled works with Pydantic 2.