python-attrs / cattrs

Composable custom class converters for attrs, dataclasses and friends.
https://catt.rs
MIT License
779 stars 108 forks source link

A dictionary gets structured into a list without errors/warnings #504

Closed fjarri closed 1 month ago

fjarri commented 4 months ago

Description

>>> cattrs.structure({"a": 1, "b": 2}, List)
['a', 'b']

I would expect it to complain that the dictionary it's trying to structure is not a list. While I understand where this logic may be coming from (a dictionary is an Iterable, so it's fine to convert it to a list), it is losing information this way, which doesn't seem like a good choice for the default behavior. Also the docs don't list Iterable as something that gets structured to a list, and dictionaries are not Sequence.

Is it intentional, and if yes, is there a way to make it more strict, in a way that's composable (that is, so I don't have to define a custom structuring function for list of every type I have)?

Tinche commented 4 months ago

Howdy!

You're right, the structure hook for lists (and sequences, mutablesequences, homogenous tuples) is very simple; you can see it here. In the case of no detailed validation, it boils down to a single list comprehension. This is for a comple of reasons:

Also the docs don't list Iterable as something that gets structured to a list

We don't actually check if the input type is an iterable. We just iterate over it. But I get your point.

is there a way to make it more strict

This is a very good question. In my mind the core job of cattrs is for this to be easy to do.

Here's how you would do this today (let's say you decide to perform an additional validation step with an isinstance against a Sequence):

from collections.abc import Sequence

from cattrs import Converter
from cattrs._compat import is_sequence

c = Converter()

def seq_hook(val, type):
    if not isinstance(val, Sequence):
        raise ValueError(f"Not a {type}")
    return c._structure_list(val, type)

c.register_structure_hook_func(is_sequence, seq_hook)

c.structure({}, list[int])

This will be composable: it will affect all lists. That said it kinda sucks.

These are all pretty easy fixes and will make the library materially better, so I'm going to assign this issue to the next release to ensure these get done.

fjarri commented 4 months ago

Thank you for a detailed response! It may also be worth noting in the docs that all Iterables and not just Sequences are structured into lists by default.