Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

immutable Mapping hooks #555

Closed
salotz opened this issue Jul 23, 2024 · 4 comments · Fixed by #556
Closed

immutable Mapping hooks #555

salotz opened this issue Jul 23, 2024 · 4 comments · Fixed by #556
Milestone

Comments

@salotz
Copy link

salotz commented Jul 23, 2024

  • cattrs version: 23.2.3
  • Python version: 3.11
  • Operating System: Linux

Description

I am bringing in an immutable mapping data type (i.e. immutables.Map https://github.com/MagicStack/immutables) into my code.

I think given that issubclass(immutables.Map, Mapping == True the is_mapping predicate should work for it and there is really no special constructor for it.

What I Did

My first attempt was to register these hooks:

from immutables import Map as frozenmap
...
converter.register_structure_hook(frozenmap, lambda d, t: frozenmap(d))
converter.register_unstructure_hook(frozenmap, lambda d: dict(d))

Which works for a plain frozenmap annotation, but not for generic type specifiers. I can write the hook factory/predicate, but I don't see why the is_mapping code should be specific to mutable mappings:

def is_mapping(type):
return (
type in (dict, Dict, TypingMapping, TypingMutableMapping, AbcMutableMapping)
or (
type.__class__ is _GenericAlias
and is_subclass(type.__origin__, TypingMapping)
)
or (
getattr(type, "__origin__", None)
in (dict, AbcMutableMapping, AbcMapping)
)
or is_subclass(type, dict)
)

@salotz
Copy link
Author

salotz commented Jul 23, 2024

For anyone curious of my solution in the meantime:

from typing import Type, Any, get_origin, TypeVar, Callable
import inspect

# Third Party Library
from cattrs import BaseConverter
from immutables import Map as frozenmap

T = TypeVar("T")
U = TypeVar("U")


def frozenmap_predicate(
        typ: Type[Any],
) -> bool:

    if inspect.isclass(typ):
        return issubclass(typ, frozenmap)
    elif is_generic_container_type(typ):
        return get_origin(typ) is frozenmap
    else:
        return False
    
def frozenmap_unstructure_factory(typ: Type[frozenmap[T,U]]) -> Callable[[frozenmap[T,U]], dict[T,U]]:
    return lambda d: dict(d)
    
def frozenmap_structure_factory(typ: Type[frozenmap[T,U]]) -> Callable[Any, Type[dict[T,U]], frozenmap[T,U]]:
    return lambda d, t: frozenmap(d)

def register_frozenmap_hooks(converter: BaseConverter) -> None:

    converter.register_structure_hook_factory(
        frozenmap_predicate,
        frozenmap_structure_factory,
    )
    converter.register_unstructure_hook_factory(
        frozenmap_predicate,
        frozenmap_unstructure_factory,
    )

@Tinche
Copy link
Member

Tinche commented Jul 23, 2024

The predicate being used here is:

def is_mapping(type):
and
def is_mapping(type):
(for 3.8).

Can these be improved to also work for immutables?

@Tinche Tinche linked a pull request Jul 23, 2024 that will close this issue
@salotz
Copy link
Author

salotz commented Jul 23, 2024

Can these be improved to also work for immutables?

I would think so via handling of ABCs. I'll check over this new documentation. I can't promise being able to write an MR anytime soon though.

In general though perhaps there is a way to register standard hooks for anything satisfying subclass relations to collections.abc base classes, e.g. if issubclass(typ, Mapping) then hooks would be basic dict conversion. It might need to be an opt-in strategy but would reduce boilerplate for users.

@Tinche
Copy link
Member

Tinche commented Jul 23, 2024

I also use immutables so I'm interested in getting this working. I started a PR to play around with this over the next couple of days.

@Tinche Tinche added this to the 24.1 milestone Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants