-
Notifications
You must be signed in to change notification settings - Fork 36
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
allow Feature **id** to be either a String or a Number #91
Conversation
I just realized this could use "Strict" types instead of dealing with smart union. Could even say that is more correct if the goal is to strictly enforce the GeoJSON spec. With the current from pydantic import StrictInt, StrictStr
...
id: Optional[Union[StrictInt, StrictStr]] = None And yes, v2 of pydantic is supposedly just around the corner, but I think this could be the right route to go even once v2 is out. Probably worth a look to see if there is other places where use of "Strict" could be useful as well. With smart union you can still do weird stuff. Especially since >>> class Foo(BaseModel):
... bar: int | str
... class Config:
... smart_union = True
...
>>> Foo(bar=3)
Foo(bar=3)
>>> Foo(bar="a")
Foo(bar='a')
>>> Foo(bar=False)
Foo(bar=False)
>>> Foo(bar=3.14159)
Foo(bar=3)
# Flipping order
>>> class Foo(BaseModel):
... bar: str | int
... class Config:
... smart_union = True
...
# The rest are the same
>>> Foo(bar=3.14159)
Foo(bar='3.14159') Whereas with "Strict" types, they have to be the exact types. >>> class Foo(BaseModel):
... bar: StrictInt | StrictStr # Order doesn't matter here
...
>>> Foo(bar=3)
Foo(bar=3)
>>> Foo(bar="a")
Foo(bar='a')
# Both of these produce the same error
>>> Foo(bar=3.14159)
>>> Foo(bar=False)
# Which I think is probably the most accurate enforcement of the spec
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "pydantic/main.py", line 341, in pydantic.main.BaseModel.__init__
pydantic.error_wrappers.ValidationError: 2 validation errors for Foo
bar
value is not a valid integer (type=type_error.integer)
bar
str type expected (type=type_error.str) |
🙏 thanks @eseglem, will switch to strict |
cc9dbed
to
4a0f90a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM whenever it stops complaining about stuff.
🤦 I messed up my branch sorry |
4a0f90a
to
5a73032
Compare
According to the specs the id is a |
closes #90
I'm a bit hesitant on this because pydantic smart-union is slower https://docs.pydantic.dev/usage/model_config/#smart-unionMaybe we can wait for pydantic v2 🤷