pydantic / pydantic-core

Core validation logic for pydantic written in rust
MIT License
1.42k stars 240 forks source link

`Sequence` Validator #133

Open samuelcolvin opened 2 years ago

samuelcolvin commented 2 years ago

Like the other sequence like validators, but keep the type.

What do we do about str? I guess we have to allow it, but there are regularly scenarios where you want "any sequence other than a string".

PrettyWood commented 2 years ago

I reckon adding a field allow_string: bool with default true ?

samuelcolvin commented 2 years ago

I'm amazed by what I wrote. I almost never want to allow str for something like this.

Up to you, but I would suggestion default false.

samuelcolvin commented 2 years ago

I've thought about this and made a start on it in #182, but I don't actually think a Sequence type is very useful.

To match python's definition of Sequence (which it kind of has to do to make any sense) it needs to allow:

But not allow (among other things):

I don't think this type is actually very helpful and would be very complicated to implement correctly.

I'll leave this open to continue the discussion when someone (inevitably) demands this.

caniko commented 2 years ago

@samuelcolvin maybe this calls for a new set of types that makes sense for Pydantic

samuelcolvin commented 2 years ago

@caniko can you give an example of what you mean?

caniko commented 2 years ago

I feel like I might not be invested enough to the discussion, but from what I gather the point of using Sequence in the first place is to loosely accept classes that fit the category. Sequence, however, is not designed for pydantic, which is why it doesn't make sense here.

Hence my idea to implement a Sequence-like type, and other types that makes sense, for Pydantic.

samuelcolvin commented 2 years ago

We could do, that but really that would effectively be a union of list | tool | set or whatever. In which situation, it would be better to be explicit and just use that union.

it's not quite the same as the union because it would be marginally faster to have an "any of these list-like types" type than use the standard union syntax, but I'm not sure, given how much it would be used, that it would be worth ti.

caniko commented 2 years ago

I think those using Sequence would appreciate it :sweat_smile:

FYI: I use it, but not with pydantic

samuelcolvin commented 2 years ago

it

I'm not very clear, appreciate what?

I find it hard to imagine many scenarios where I would really want Sequence[T].

caniko commented 2 years ago

I'm not very clear, appreciate what?

Appreciate adding something that is homologous to Sequence designed for pydantic.

I find it hard to imagine many scenarios where I would really want Sequence[T].

What about those few scenarios?

This is not my fight, I'll let someone with a real problem pick up the Yea side arguments from here.

Opinion: I am sure there is some niche use of these loose types for pydantic; I haven't stumbled over this yet. Moreover, typing.Sequence is also quite niche IMHO.

sydney-runkle commented 2 months ago

I'm in favor of this, see https://github.com/pydantic/pydantic/issues/10036