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

List[T] validation is not strict enough #86

Closed
mgu opened this Issue Oct 21, 2017 · 13 comments

Comments

4 participants
@mgu
Contributor

mgu commented Oct 21, 2017

with this kind of model :

class Foo(BaseModel):
    items: List[int]

Foo(items='') does not throw any error. Looking at the code, we do not check the list is a list, but we iterate over it and check the elements. Any empty iterable can be used for List[T]

@samuelcolvin

This comment has been minimized.

Owner

samuelcolvin commented Oct 21, 2017

Not at my computer right now, will reply next week. But there's already an issue discussing this I think.

@samuelcolvin

This comment has been minimized.

Owner

samuelcolvin commented Oct 23, 2017

There's a long discussion about type casting on vs. raising an error on #45.

I think this is the correct behaviour. Pydantic tries to follow python on what can be cast to what: since '123' can be converted to a list and each element can be converted to an int, it makes sense (at least in my head) that '123' can be converted to [1, 2, 3]. '' is just a special case of this.

@mgu

This comment has been minimized.

Contributor

mgu commented Oct 23, 2017

For me there is a difference between typing.List and typing.Sequence
'123' is a Sequence, not a List

@samuelcolvin

This comment has been minimized.

Owner

samuelcolvin commented Oct 23, 2017

I understand that, but pydantic generally relies on "does list(value) work?" and list('123') does work.

The point is that the type hinting is used to define what type the deserialised value will be, not what type input data is.

I won't be changing the libraries standard functionality, but you're welcome to implement StrictList with your own checks.

@mgu

This comment has been minimized.

Contributor

mgu commented Oct 23, 2017

understood, thx.

@pawelswiecki

This comment has been minimized.

pawelswiecki commented Jun 11, 2018

I think this is unexpected:

from typing import List
from pydantic import BaseModel


class User(BaseModel):
    values: List[str]

data = {'values': 'a_string'}

user = User(**data)
assert user.values == ['a', '_', 's', 't', 'r', 'i', 'n', 'g']

Maybe make an exception for strings and don't treat them as sequences, at least in some contexts?

mypy is having the same problem, see python/mypy#5090.

@samuelcolvin

This comment has been minimized.

Owner

samuelcolvin commented Jun 11, 2018

Ok, for this case I'm happy to accept a change which checks if value is str or bytes and raises a type error. PR welcome.

@samuelcolvin samuelcolvin reopened this Jun 11, 2018

@Gr1N

This comment has been minimized.

Collaborator

Gr1N commented Jun 12, 2018

Why we want to check if value is str or bytes only? What about dict?

In [1]: from pydantic import BaseModel

In [2]: from typing import List

In [3]: class Model(BaseModel):
   ...:     foo: List[str]
   ...:

In [4]: m = Model(foo={'key': 'value'})

In [5]: m.foo
Out[5]: ['key']

In [8]:

Maybe better to go with logic similar to StrictStr? Or just check if value not is instance of list and raise exception?

@samuelcolvin

This comment has been minimized.

Owner

samuelcolvin commented Jun 13, 2018

I guess list or tuple, but nothing else?

@Gr1N

This comment has been minimized.

Collaborator

Gr1N commented Jun 13, 2018

What about set?

@samuelcolvin

This comment has been minimized.

Owner

samuelcolvin commented Jun 13, 2018

yes.

Gr1N added a commit to Gr1N/pydantic that referenced this issue Jun 13, 2018

@Gr1N

This comment has been minimized.

Collaborator

Gr1N commented Jun 13, 2018

@samuelcolvin review please #200

samuelcolvin added a commit that referenced this issue Jun 13, 2018

Strict validation of `list`, `set` and `tuple` (#86) (#200)
* Strict validation of `list`, `set` and `tuple` (#86)

* review fixes
@Gr1N

This comment has been minimized.

Collaborator

Gr1N commented Jun 13, 2018

@samuelcolvin can we close this issue? Fix merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment