we don't technically need to compare both the value if we know they're the same type and the same name... although maybe this is more clear

Good point, removed comparison of value

Could you add a test for this ? you can test this with isinstance

def foo(x: Any)
    if isinstance(x, Color):
        return x.value
    ...

Done. To support this test case, I also had to implement a bit of code in pybind_utils.h tryToInferType. Please also take a look there.

this is being used for exporting enum constants (thus the annotated printing of the dict), so i'm not about the final ( v.value() ).

Good point, removed.

nit, i would maybe do type->cast<EnumType>(), as it would be weird if we found a AnyEnumType here. All of the runtime types should EnumType, and the AnyEnumType should just used for schema matching.

Good point. Done.

don't need to ignore loc, it's used below. in general. from searching around, looks like std::ignore is only ever used in this code as part of std::tie (the clang tidy errors you maybe seeing dont get run on master)

Done. Removed std::ignore

it might be easier to change these 4 lines to: py::module::import("enum").attr("Enum"), I don't think there error messages we're giving here are any better than what python would throw.

Done.

nit: is there a reason to not return PyObject_IsSubclass(obj.ptr(), enum_type_obj) ?

It may return -1, which indicates failure to compare, and -1 becomes True in boolean.

Is it possible here instead of getting Color.Red, we're getting Color.Red.value

Discussed with eellison offline and addressed misunderstanding. This is WAI.

this code path gets executed when a python value gets closed over in a script function.

def foo():
     a = Color.Red
    return 

At this point, we may not necessarily have compiled Color as an Enum Class yet. I think you need to pass in the type of the object into the try_ann_to_type so that we ensure the class is regsitered

Done. Using try_ann_to_type here instead of building up the type directly here.

don't need to do it as part of this PR, but if you return a list of all of the Enum values this will work

Ack, it is already in next PR.

I would comment out the function parameter if it's not used.

Done.

Done.

Can you add some test cases for the case we discussed offline, where an Enum class isnt used as a type annotation and closed over?

a = Color
def foo():
    a

b = Color.Red
def foo():
    b...

Done.

I think we can just compile the type, bc the type stores the name->value mapping, and use that

I think this has the same problems as we did previously, where we're individually introspecting the object's value so the EnumType doesn't get registered and we won't hit a compile error if someone tries to use an Enum with heterogenous types.

Done.

A test to add this time or next time (I think this should work right now) is putting an Enum as a attribute of a module and check that it is retained durign scripting
`
class Mod(nn.Module):
self.a = Color.RED

def forward(self):
return self.a

Done.