Skip to content

Fix creation of graphene.Enum from enum.Enum #154

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

Merged
merged 4 commits into from Oct 26, 2018
Merged

Fix creation of graphene.Enum from enum.Enum #154

merged 4 commits into from Oct 26, 2018

Conversation

wichert
Copy link
Contributor

@wichert wichert commented Aug 3, 2018

This fixes #152, and adds a test to make sure it keeps working :)

@wichert
Copy link
Contributor Author

wichert commented Aug 3, 2018

There is a bit of an issue here, which was causing the tests to fail: using Enum.from_enum() makes the conversion to and from PyEnum works correctly, but it looses the name from the SQLAlchemy enum type and instead uses the name from the PyEnum type. I would love to argue that that is the Right Thing To Do ™️, but I can see it being a surprise to people. Fixing it probably means extending graphene to support a name parameter to Enum.from_enum().

@syrusakbary, can you weigh in on this?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 91.877% when pulling d4365e1 on curvetips:fix-enum-conversion into 8872577 on graphql-python:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 91.877% when pulling d4365e1 on curvetips:fix-enum-conversion into 8872577 on graphql-python:master.

@stan-sack
Copy link

stan-sack commented Aug 4, 2018

This PR does not fix my graphene.InputObjectType which was defined as:

class PetInput(graphene.InputObjectType):
    ...
    hairKind = graphene.Enum.from_enum(models.HairKind)
    ...

The InputObjectType above works in v2.0.0 but fails with error: graphql.error.base.GraphQLError: Expected a value of type "hairKind" but received: HairKind.LONG, even after pulling this PR.

@wichert
Copy link
Contributor Author

wichert commented Aug 4, 2018

@stan-sack Is that another regression in v2.1.0? It looks like there are no tests for enum inputs, so we'll need to add those I guess.

@stan-sack
Copy link

@wichert . Yeah I think so. Thanks. I love the library. Its great to see that its being actively maintained. It looks like Enums have been a point of discussion for some time now.

@sebastiandev
Copy link

We had an issue today and this worked, it only needs an extra change from my PR

@convert_sqlalchemy_type.register(types.Enum)
def convert_enum_to_enum(type, column, registry=None):
    enum_class = getattr(type, 'enum_class', None)

    if enum_class:  # Check if an enum.Enum type is used
        graphene_type = Enum.from_enum(enum_class)

    else:  # Nope, just a list of string options
        items = zip(type.enums, type.enums)
        graphene_type = Enum(type.name or "{}_{}".format(column.table, column.name), items)

    return Field(
        graphene_type,
        description=get_column_doc(column),
        required=not (is_column_nullable(column)),
    )

@kousun12
Copy link

@wichert any chance we can get this in soon?

@wichert
Copy link
Contributor Author

wichert commented Oct 26, 2018

@kousun12 That is up to @syrusakbary - this is how project. Personally I ended up reevaluating our use of GraphQL and graphene and moved back to REST.

@syrusakbary
Copy link
Member

My fault. These last months had been frenetic personally for me (new company creation, new immigrant visa to be able to stay in the US, ...) and it had occasioned some downsides/side-effects to Graphene.
Sorry about that, things will start getting into shape soon.

PS: To everyone using the project. If you are using it commercially, consider donations as a way to support it, so it can become a sustainable project: https://www.patreon.com/syrusakbary

@syrusakbary syrusakbary merged commit 6a96d37 into graphql-python:master Oct 26, 2018
@Nabellaleen Nabellaleen added this to the 2.1.1 milestone Mar 29, 2019
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 this pull request may close these issues.

Serializing native Python enums does not work
7 participants