Skip to content
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

Changes to list, tuple, set, frozenset coersion #191

Closed
samuelcolvin opened this issue Jul 24, 2022 · 10 comments · Fixed by #208
Closed

Changes to list, tuple, set, frozenset coersion #191

samuelcolvin opened this issue Jul 24, 2022 · 10 comments · Fixed by #208
Labels
enhancement New feature or request

Comments

@samuelcolvin
Copy link
Member

Stop coercing set / frozenset to list / tuple?

Although this is not "loosing information", the result is not deterministic/repeatable.

E.g. if you have the Field Tuple[PositiveInt, NegativeInt, str] then the input set {1, -1, 'a'} will work sometimes, and fail sometimes - this is pretty confusing.

I think we should change this.

Stop coercing list / tuple to set / frozenset?

Should we allow coercing a list to a set? In this case we are "loosing information" (e.g. order), however creating a set from a list is often desired - e.g. when parsing a format (yaml, toml etc.) that only has a list type.

I think we should not change this

Add coercing dict_key to set / frozenset?

Not that common, but we have it now and I think it kind of makes sense since dict_key "feels like" (sorry to be fluffy) a set.

I guess since dict_key are ordered, it should be fine to coerce them to list and tuple too.

I guess as with currently, we should allow dict_values to all these types too?

I think we should change this.

Generators?

In pydantic V1 we allow converting a generator to any of these types.

I think we should allow converting a generator to a list or tuple, but not set or frozenset.

@PrettyWood @tiangolo thoughts?

@samuelcolvin samuelcolvin added the enhancement New feature or request label Jul 24, 2022
@maestro-1
Copy link

Looks like something I would be interested in handling @samuelcolvin

@maestro-1
Copy link

Question though, sure a little bit of information seems to be lost when converting a generator to a set(order), but the order is very rarely considered when working with generators, as for the most part, you just need a one-time iterable. So is there really any reason(argument) for refusing coercing to frozenset/sets

@samuelcolvin
Copy link
Member Author

Looks like something I would be interested in handling @samuelcolvin

Thanks, yes please.

I think order is definitely considered when working with generators quite often, but since I'm proposing lists can be converted to sets, I guess it also makes sense to allow generators.

Still would love input from others.

The only thing here that I'm sure about is preventing sets from being converted to lists/tuples.

@PrettyWood
Copy link
Member

the order is very rarely considered when working with generators

Really? I would have said the opposite. Personally I almost always consider the order.

Stop coercing set / frozenset to list / tuple?

Yep completely agree. If people want to force coercion, they can always use a validator.

Stop coercing list / tuple to set / frozenset?

Hmmm yeah I guess it makes sense with json parsing for example. But I would have understood if coercion was only done if len(set(v)) == len(v) for example or even if coercion was not done by default. This would be a bit stricter but more consistent with the "we coerce only if we don't lose information".

Add coercing dict_key to set / frozenset?

Yeah dict_key is some basically an ordered set.

If we had an OrderedSet type, we could be more consistent with the coercion

@samuelcolvin
Copy link
Member Author

it makes sense with json parsing for example

Well, we can cover JSON parsing separately because of the JSON Input type. But still on balance I think it makes sense.

I think we need to either add two clauses to the cod-philosophy theory of pydantic type coercion:

  • If coercion of the input type to the output type is not deterministic, throw an error. HARD RULE - never broken.
  • we're more permissive when downcasting than upcasting - or more generally - we're more willing to coerce to a more specific less common type from a more general common type than visa versa - e.g. str -> Path, list -> set. SOFT RULE.

@maestro-1
Copy link

I think we need to either add two clauses to the [cod-philosophy theory of pydantic type coercion](https://pydantic-docs.helpmanual.io/blog/pydantic-v2/#formalised-conversion-table):
Think that would be really helpful to know what is allowed and what isn't. If the decision to allow the coerce of list to set remains, then a reason will more than likely be needed for why the inverse isn't acceptable(done by default).

@samuelcolvin
Copy link
Member Author

samuelcolvin commented Jul 25, 2022

@maestro-1 see here, if you want to contribute to pydantic it would be worthwhile reading that whole article.

@maestro-1
Copy link

maestro-1 commented Jul 25, 2022

@samuelcolvin Already gone through it. Still have it open even for when I need to reference anything

@PrettyWood
Copy link
Member

I didn't mean to cover JSON parsing separately. Better to have one rule for all.

I think we need to either add two clauses to the cod-philosophy theory of pydantic type coercion:

  • If coercion of the input type to the output type is not deterministic, throw an error. HARD RULE - never broken.
  • we're more permissive when downcasting than upcasting - or more generally - we're more willing to coerce to a more specific less common type from a more general common type than visa versa - e.g. str -> Path, list -> set. SOFT RULE.

👍 As long as the rule is clear and explicit it's great

@PrettyWood
Copy link
Member

In fact the sentence If the input data has a SINGLE and INTUITIVE representation here explains already why sets should not be coerced to lists for example: since {1, 2, 3} is not the only representation, {2, 1, 3} is also valid


@samuelcolvin I opened #208
IMO if list -> set is allowed, generator -> set should be allowed too (iterators). We can start by tackling those two points on which we seem to agree. Other changes on coercion can always be made afterwards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants