cattrs icon indicating copy to clipboard operation
cattrs copied to clipboard

Add support for pathlib.Path

Open spacemanspiff2007 opened this issue 4 years ago • 10 comments

  • cattrs version: 0.9.0
  • Python version: 3.7
  • Operating System: Win

Description

I am trying to serialize a class with a pathlib.Path object

What I Did

from pathlib import Path
import attr

@attr.s
class PathTest():
    my_path: Path = attr.ib(default=Path('c:/test'))

cattr.structure({'my_path': 'c:/my_filepath',}, PathTest)

Error:

ValueError: Unsupported type: <class 'pathlib.Path'>. Register a structure hook for it.

The fix is easy, but I really feel like this is a fundamental type and should be supported out of the box

cattr.register_structure_hook(Path, lambda d, t: Path(d))
cattr.register_unstructure_hook(Path, lambda d: str(d))

spacemanspiff2007 avatar Dec 17 '19 12:12 spacemanspiff2007

This would be very nice for GenConverter, I'd be willing to take a stab at implementing if this is deemed a good idea. I think it could be cattr.register_unstructure_hook(Path, os.fspath).

Edit: actually, most types don't have converters except for the preconfigured ones, maybe it wouldn't fit with the design?

henryiii avatar Mar 16 '22 18:03 henryiii

I'm not sure this is useful enough to be included by default, but a recipe in the Usages section of the docs would be cool!

Tinche avatar Mar 24 '22 14:03 Tinche

@Tinche , I just stumbled over this too. I think support for Pathlib would be very useful, because the way I understand it, pathlib is supposed to be a modern replacement for os.path.

Oh and thx @spacemanspiff2007 for the fix.

rklasen avatar Aug 02 '22 13:08 rklasen

Two things:

  • I don't want cattrs to be a kitchen-sink type of library, since it increases complexity and maintenance burden
  • I do want cattrs to be a kind of meta-framework, i.e. a tool that you use as a base to create your own mini-framework for de/serialization

More fundamentally, this approach is also how I think good software should be architected.

As the OP rightly points out, adding support for pathlib.Path is literally two lines of code. So the way we handle this is we add a small section to the Usage Examples page so our users can include this support in their converters if they want it, and learn a little cattrs in the process. I'd merge this PR in a heartbeat.

Tinche avatar Aug 05 '22 17:08 Tinche

I absolutely agree with you: Support for every datatype will unnecessarily inflate cattrs.

However pathlib.Path is the recommended way to deal with file paths and it's part of the python standard installation which makes it a bit different. It is like Enum, which you support out of the box, too and is also not a built-in datatype.

spacemanspiff2007 avatar Aug 06 '22 04:08 spacemanspiff2007

Alright, I consuled some other folks and I've changed my mind. Let's add this.

Tinche avatar Aug 07 '22 21:08 Tinche

Thank you, I really appreciate this!

rklasen avatar Aug 09 '22 08:08 rklasen

Awesome. I use Path everywhere ;-)

BTW, mypy produces the following message.

cattr.register_unstructure_hook( Path, lambda d: str(d) )

"Cannot infer function type argument"

Any suggested "fixes"?

brendan-simon-indt avatar Aug 22 '22 23:08 brendan-simon-indt

This is just a lack of lambda supports for type inference right? a workaround would look something like:

_convert_str: t.Callable[[t.Any], str] = lambda d: str(d)

cattr.register_unstructure_hook(Path, _convert_str)

aarnphm avatar Aug 23 '22 00:08 aarnphm

Yep, that works. I love the way Python is supposed to be pseudocode like and easy to read and understand 😉 🙄

My lazy workaround was:

cattr.register_unstructure_hook( Path, lambda d: str(d) )       # type: ignore

🤣

brendan-simon-indt avatar Aug 23 '22 01:08 brendan-simon-indt

Fixed on main!

Tinche avatar Jan 03 '23 22:01 Tinche