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

Support for Enums #73

Open
RazCrimson opened this issue Apr 26, 2024 · 14 comments
Open

Support for Enums #73

RazCrimson opened this issue Apr 26, 2024 · 14 comments

Comments

@RazCrimson
Copy link

Currently Inline snapshot doesn't seem to support working with Enums.

Example:

from enum import Enum

from inline_snapshot import snapshot


class SomeEnum(Enum):
    VAL1 = "val1"


def test_snap():
    a = SomeEnum.VAL1
    assert a == snapshot()

Running pytest --inline-snapshot=create states that the snapshot were defined, but they are actually not. I presume this is due to the behavior of repr with Enums:

repr(SomeEnum.VAL1)
"<SomeEnum.VAL1: 'val1'>"

I am not sure as to how it can be handled, but maybe some regex filtering could be applied over reprs of only Enum objects and just use the Enum value for the snapshot. Would really love this.

P.S. I am not really confident, but I'll try taking a stab at this if you fine with accepting a PRs.

@15r10nk
Copy link
Owner

15r10nk commented Apr 27, 2024

I think that the regex approach might become problematic. It might work for this case but there are maybe usecases in the future where the repr string is missing some information.

A custom repr implementation which you can specialize for your type with a generic fallback to repr() might be a better solution.

a singledispatch might be useful here.

@15r10nk
Copy link
Owner

15r10nk commented Apr 27, 2024

I can do a first implementation today in the evening.

@RazCrimson
Copy link
Author

Just some thoughts.

I just mentioned plain enums above, but my actual usecase of enums is within pydantic objects. I haven't not worked with singledispatch before this, but taking a quick look at the documentation I think it works only with plain types and not with class hierarchies.

So if you planning to add support to use custom repr for certain types, then I think that the dispatcher function needs to somehow identify container objects like lists, dicts, tuples, pydantic objects and individually call the dispatcher on the items contained. That would make enums work out of the box with no changes required by the users.

But I am not sure on what we could do to handle objects with user/client specified custom repr with enums in them.

@15r10nk
Copy link
Owner

15r10nk commented Apr 27, 2024

did you wrote the enum class or is it from a other library?

you could do the following, if it is your own.

from enum import Enum

from inline_snapshot import snapshot


class SomeEnum(Enum):
    VAL1 = "val1"

    def __repr__(self):
        return type(self).__name__ + "." + self._name_


def test_snap():
    a = SomeEnum.VAL1
    assert a == snapshot(SomeEnum.VAL1)

@15r10nk
Copy link
Owner

15r10nk commented Apr 27, 2024

another more generic approach might be this

from enum import Enum

from functools import singledispatch
from pydantic import BaseModel


class SomeEnum(Enum):
    VAL1 = "val1"


a = SomeEnum.VAL1


class PydanticContainer(BaseModel):
    v: SomeEnum


class Container:
    def __init__(self, v):
        self.v = v

    def __repr__(self):
        # this does not work, because the f-string does no lookup of repr in the global/builtin namespase
        # return f"{self.v}"

        # repr from __builtins__ is our code_repr
        # this means that a __repr__ which uses repr()
        return repr(self.v)


real_repr = repr


@singledispatch
def code_repr(v):
    return real_repr(v)


@code_repr.register
def _(v: Enum):
    return type(v).__name__ + "." + v._name_


@code_repr.register
def _(v: list):
    return "[" + ",".join(map(repr, v)) + "]"


# this would be only changed when we convert the value to source code
__builtins__.__dict__["repr"] = code_repr

# does work for a simple Container which uses repr()
print(repr([Container(a), a, 5]))

# does not work for PydanticContainer out of the box,
# because pydantic does not look up our mocked repr
print(repr(PydanticContainer(v=a)))

# special handling for pydantic BaseModel which uses repr()
@code_repr.register
def _(model: BaseModel):
    return (
        type(model).__name__
        + "("
        + ", ".join(
            e + "=" + repr(getattr(model, e)) for e in model.__pydantic_fields_set__
        )
        + ")"
    )

# now it works
print(repr(PydanticContainer(v=a)))

output (Python 3.12.1):

[SomeEnum.VAL1,SomeEnum.VAL1,5]
PydanticContainer(v=<SomeEnum.VAL1: 'val1'>)
PydanticContainer(v=SomeEnum.VAL1)

But I would like to know if it is possible for you to add a __repr__ to your Enum (comment above), because this might be the easiest solution here.

@RazCrimson
Copy link
Author

RazCrimson commented May 3, 2024

did you wrote the enum class or is it from a other library?

Some of them are under my control and some are not. I could make the enums under my control to have the custom repr. But how should I go about patching the ones from other libraries?

Is there any way I could tell inline-snapshot to use a custom repr function instead of plain repr or how should I go about doing that?

@15r10nk
Copy link
Owner

15r10nk commented May 3, 2024

Some of them are under my control and some are not. I could make the enums under my control to have the custom repr. But how should I go about patching the ones from other libraries?

yes, this would become a never ending story. Allowing to customize repr() for specific types (like in my code example) might be a good idea.

This has the drawback that I also have to provide custom repr() implementations for Container types like pydantic.BaseModel (if they are not using repr() in there __repr__() implementation or are implemented in C and use PyObject_Repr)

Enums would be supported by default, also inside a pydantic.BaseModel.

Is there any way I could tell inline-snapshot to use a custom repr function instead of plain repr or how should I go about doing that?

You will also be able to register custom repr() implementations for specific types.
something like:

from inline_snapshot import register_repr

@register_repr
def _(obj: MyCustomType):
    return "MyCustomType(...)"

This is currently only a plan and not implemented. But I'm confident that it will work.

15r10nk added a commit that referenced this issue May 3, 2024
15r10nk added a commit that referenced this issue May 6, 2024
@15r10nk
Copy link
Owner

15r10nk commented May 6, 2024

hi, @RazCrimson you can test the #85 if you want.
I think it should work for your use case and you can use it now if you want.
I only want to test it a bit more before I release this feature.

@RazCrimson
Copy link
Author

Cool, I'll test it out tomorrow morning and let you know if I find any issues.

15r10nk added a commit that referenced this issue May 7, 2024
15r10nk added a commit that referenced this issue May 20, 2024
@ttrei
Copy link

ttrei commented May 21, 2024

I also stepped on this issue and traced it to Enum after some debugging.

Maybe we can report something to the user if the parsing fails here?

try:
ast.parse(new_code)
except:
return

@ttrei
Copy link

ttrei commented May 21, 2024

As for #85, it didn't fix my use case of enums inside dataclasses:

from dataclasses import dataclass
from enum import Enum

from inline_snapshot import snapshot


class SomeEnum(Enum):
    VAL1 = "val1"


@dataclass
class Foo:
    e: SomeEnum


def test_snap():
    foo = Foo(e=SomeEnum.VAL1)
    assert foo == snapshot()

The resulting snapshot:
assert foo == snapshot(HasRepr("Foo(e=<SomeEnum.VAL1: 'val1'>)"))

The assertion fails (after importing HasRepr):

>       assert foo == snapshot(HasRepr("Foo(e=<SomeEnum.VAL1: 'val1'>)"))
E       assert Foo(e=<SomeEnum.VAL1: 'val1'>) == HasRepr("Foo(e=<SomeEnum.VAL1: 'val1'>)")
E        +  where HasRepr("Foo(e=<SomeEnum.VAL1: 'val1'>)") = snapshot(HasRepr("Foo(e=<SomeEnum.VAL1: 'val1'>)"))
E        +    where HasRepr("Foo(e=<SomeEnum.VAL1: 'val1'>)") = HasRepr("Foo(e=<SomeEnum.VAL1: 'val1'>)")

@15r10nk
Copy link
Owner

15r10nk commented May 21, 2024

Thank you for trying this feature branch. The good thing about the approach is that you can replace repr if you want, the bad thing is that you have to replace it sometimes if you don't want.

What is missing in your case is a custom repr implementation for data classes which recursively uses repr() for the attributes.

You can try to build one or wait a bit because I have other things to do for the next two days.

Auto import of missing names is also something I want to implement.

@15r10nk
Copy link
Owner

15r10nk commented May 23, 2024

@ttrei you issues should be fixed in #85

dataclasses are handled with a special repr() implementation

Maybe we can report something to the user if the parsing fails here?

inline-snapshot generates HasRepr(type,repr) for every object where repr() generates none parsable python code.

@ttrei
Copy link

ttrei commented May 24, 2024

@ttrei you issues should be fixed in #85

confirmed, thanks!

15r10nk added a commit that referenced this issue May 26, 2024
15r10nk added a commit that referenced this issue May 28, 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

No branches or pull requests

3 participants