python-attrs / cattrs

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

cattrs does not unstructure os.PathLike to str #417

Open intelfx opened 1 year ago

intelfx commented 1 year ago

Description

When unstructuring an attrs class where one of the fields is annotated as os.PathLike (and contains a Path), that field remains a Path (in my case, PosixPath) instead of being unstructured as str.

Curiously, annotating the field as Union[str, os.PathLike] or pathlib.Path (without changing the actual data type) resolves the issue.

What I Did

In [1]: import os
   ...: from pathlib import Path
   ...: from typing import Union
   ...: 
   ...: import attrs
   ...: import cattrs
   ...: import cattrs.preconf.json

In [2]: @attrs.define
   ...: class MyClass:
   ...:     foo: Union[str, os.PathLike] = Path("/foo")
   ...:     bar: os.PathLike = Path("/bar")
   ...: 

In [3]: cattrs.preconf.json.make_converter().unstructure(MyClass())
Out[3]: {'foo': '/foo', 'bar': PosixPath('/bar')}

In [4]: cattrs.unstructure(MyClass())
Out[4]: {'foo': '/foo', 'bar': PosixPath('/bar')}

I'm not saying that this must be a bug (rather than merely my mis-usage of cattrs), but I was not able to find anything in the docs that would suggest that I was doing something incorrectly.

Tinche commented 1 year ago

We don't support os.PathLike, no one has asked for it yet. pathlib.Path is supported though.

When generating the unstructuring code, cattrs looks at the type of the field in the class rather than the actual runtime value of the field. If cattrs doesn't know how to unstructure a field, it just passes it through, which is what you're seeing. (For structuring it's different, there it would be an exception.)

To solve the issue, you can register a super simple hook yourself:

c.register_unstructure_hook(PathLike, str)
intelfx commented 1 year ago

If cattrs doesn't know how to unstructure a field, it just passes it through, which is what you're seeing.

I see. This was not obvious from the documentation.

Perhaps there is value in a "strict mode" that would ensure that either everything is unstructured to primitive types or an exception is thrown? Such a mode could then be auto-enabled for the preconfigured converters.

Tinche commented 1 year ago

I can consider it, but what's the use case? Either cattrs raises something or your json lib raises something, feels like it ends up being the same?