python-attrs / cattrs

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

JSON serialisation fails #453

Closed Ttayu closed 7 months ago

Ttayu commented 7 months ago

Description

Please refer this. https://github.com/python-lsp/python-lsp-ruff/issues/61

I'm guess it may be that serialisation fails when dataclass is included in cattrs==23.2.1

What I Did

Paste the command(s) you ran and the output.
If there was a crash, please include the traceback here.
Tinche commented 7 months ago

Looks like this is a regression introduced in https://github.com/python-attrs/cattrs/pull/381 for a type annotated as Any | None.

Might need to contemplate this a little since it's not very clear to me what the correct behavior should be.

jvesely commented 7 months ago

This issue still appears to be present in 23.2.2

Tinche commented 7 months ago

Any chance of a small reproducer? OP has confirmed their issue is solved on 23.2.2 in a different ticket.

kmantel commented 7 months ago

Here's one.

import json
from typing import Any, Dict, Optional

import attr
import cattr
import numpy as np

def _unstructure_value_expr(o):
    try:
        return o.tolist()
    except AttributeError:
        return converter.unstructure(o)

converter = cattr.Converter()
converter.register_unstructure_hook(np.ndarray, _unstructure_value_expr)

@attr.define
class A:
    dict_field: Optional[Dict[str, Any]] = attr.field()

a = A({'arr': np.array([0])})
print(json.dumps(converter.unstructure(a)))

This gives TypeError: Object of type ndarray is not JSON serializable on 23.2.1 and 23.2.2, but gives correct output on 23.1.1 and 23.1.2

Changing the dict_field line to dict_field: Optional[Dict[str, np.ndarray]] = attr.field() or dict_field: Dict[str, np.ndarray] = attr.field()

gives the correct output on each version.

while

dict_field: Dict[str, Any] = attr.field()

fails on all four.

This is on Python 3.11.6 but I don't think it depends on that

Tinche commented 7 months ago

Ah interesting, looks like we have another regression in handling dict values. Will check it out today.

Tinche commented 7 months ago

@kmantel can you try https://github.com/python-attrs/cattrs/tree/tin/fix-dict-any and see if it fixes your issue?

jvesely commented 7 months ago

@kmantel can you try https://github.com/python-attrs/cattrs/tree/tin/fix-dict-any and see if it fixes your issue?

@Tinche, the above link does not work but using the top of 32.2 branch (pip install git+https://github.com/python-attrs/cattrs.git@23.2) fixes the issues in psyneulink (referenced in the above commit). Is there a new release planned soon?

Tinche commented 7 months ago

Yeah, in the next day or two as I chase down a different issue.