python-attrs / cattrs

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

Structuring arbitrary types by identity ? #393

Closed philpep closed 1 year ago

philpep commented 1 year ago

Description

Hi, I've attrs models with datetime.datetime and datetime.date fields (parsed by pyyaml) which are not supported by base converter.

I wonder if we could have a default structure hook for unknown classes that return given object if its class match expected class ?

What I Did

Minimal example:

import datetime

import cattrs

cattrs.structure(datetime.date.today(), datetime.date)

Gives:

  File "lib/python3.11/site-packages/cattrs/converters.py", line 334, in structure
    return self._structure_func.dispatch(cl)(obj, cl)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "lib/python3.11/site-packages/cattrs/converters.py", line 402, in _structure_error
    raise StructureHandlerNotFoundError(msg, type_=cl)
cattrs.errors.StructureHandlerNotFoundError: Unsupported type: <class 'datetime.date'>. Register a structure hook for it.

Related to #50

Tinche commented 1 year ago

Yeah, I think we'll need something like this. https://github.com/python-attrs/cattrs/issues/278 is also a form of this I feel.

(Note that there's a preconfigured converter for pyyaml at https://github.com/python-attrs/cattrs/blob/main/src/cattrs/preconf/pyyaml.py)

I'm not super sure how to do this exactly yet. What I'm thinking is, we add a converter parameter that's essentially a set of types the underlying serialization framework can handle, and add some default hooks making use of this. These types can just be validated by checking their class.

PIG208 commented 1 year ago

I have a similar use case that might need this.

Our input comes from orjson, which is already capable of parsing the data into primitives like str, int, bool.

The default BaseConverter uses _structure_call for these types. The thing is, we don't want an int parsed from JSON to be structured into an str, just because str(123) is valid. So a possible fix for us is that the converter should just reject values that are not an instance of the primitive types handled by orjson, or more generally, allow per-type overrides.

PIG208 commented 1 year ago

I looked into type_overrides on Converter, but it seems that the current implementation only supports overriding dict fields, probably because it is only there to support make_dict_structure_fn and make_dict_unstructure_fn.

PIG208 commented 1 year ago

Just check #278, I think the snippet should be sufficient for our use case, but it will be nice to have something similar built into the pre-configured converters.

Tinche commented 1 year ago

@PIG208 That sounds like a separate thing.

If cattrs is calling str() on your field, it's because it thinks that field should be a string, probably because you marked it as a string in a class. Leaving it as an int would be an error in that case.

You mention per-type overrides, we already support several forms of this. Might be interesting to go into details here.

PIG208 commented 1 year ago

cattrs is calling str() on your field, it's because it thinks that field should be a string, probably because you marked it as a string in a class.

Right. My attrs class expects it to be a string, but I also want to get an error if the input is not already an instance of str.

Tinche commented 1 year ago

Yeah, that's a different thing. You can easily customize this:

from cattrs import Converter

c = Converter()

def validate_string(val, _) -> str:
    if not isinstance(val, str):
        raise TypeError()
    return val

c.register_structure_hook(str, validate_string)
Tinche commented 1 year ago

Alright, a small update here.

In #403, I've snuck in datetime.date support for the pyyaml converter. That PR is mostly for fancier unions though, so it will actually enable types like date | datetime | int for the pyyaml converter.

As for adding support for arbitrary single types, we can point folks to https://github.com/python-attrs/cattrs/issues/393#issuecomment-1652709454 which should be simple and powerful enough.