strawberry icon indicating copy to clipboard operation
strawberry copied to clipboard

Use @strawberry.enum value instead of name

Open BryceBeagle opened this issue 3 years ago • 32 comments

I was a bit surprised that the following code snippet prints {'userType': 'BLAH'} instead of {'userType': 'c'}

from enum import Enum

import graphql
import strawberry


@strawberry.type
class User:
    @strawberry.enum
    class UserType(Enum):
        A = "a"
        B = "b"
        BLAH = "c"

    name: str
    user_type: UserType


@strawberry.type
class Query:
    @strawberry.field
    def user(self) -> User:
        return User(
            name="Bob",
            user_type=User.UserType.BLAH
        )


schema = strawberry.Schema(query=Query)
query = "{ user { userType } }"

result = graphql.graphql_sync(schema, query)

user = result.data['user']
print(user)

Is there a way to have the response use the Enum's value instead of its name? If the name is supposed to be used, is there any reason to define values instead of using Enum.auto()?

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar

BryceBeagle avatar Jul 08 '20 16:07 BryceBeagle

yeah that's a very good question! I spoke about this with @jkimbo as he has had more experience with enums (in Graphene expecially).

The confusion comes from GraphQL not having values for enums. So we need to make a choices when creating the enum. I think it makes sense to use the name instead of the value, which, as you mentioned, could also be coming from auto() and I wouldn't really like to have a name that's autogenerated, what do you think?

patrick91 avatar Jul 08 '20 21:07 patrick91

I think by default auto() assigns an integer value to the enum. I wonder if @strawberry.enum could define a _generate_next_value_() on the wrapped Enum that takes the enum field name and uses it as the value.

Then we could allow/enforce the user to use only strings or auto() for the values. If they use strings, the value is used as the name for GraphQL; if they use auto(), the Python name is used.

BryceBeagle avatar Jul 08 '20 21:07 BryceBeagle

If they use strings, the value is used as the name for GraphQL; if they use auto(), the Python name is used.

this sounds a bit confusing.

What would be advantage of using the name? I guess we could also have an option, like this:

@strawberry.enum(name_from_value=True)
class UserType(Enum):
    A = "a"
    B = "b"
    BLAH = "c"

patrick91 avatar Jul 08 '20 21:07 patrick91

What would be advantage of using the name?

So, I wasn't aware that SCREAMING_SNAKE_CASE was the standard for GraphQL enums. Currently, the json structure I'm trying to migrate to graqhql uses snake_case pseudo-enums, and I just stuck with that. I'll probably just make changes to adhere to the standard going forward.

@strawberry.enum(name_from_value=True)

I think this could be a decent option. Now that I'm aware that there's a standard for enum naming, I wonder if it would be best to just have users always use auto() (potentially with _generate_next_value_() defined as above), unless they explicitly use the name_from_value arg. That way there's no confusion on where the GraphQL name comes from or what purpose the enum value has.

BryceBeagle avatar Jul 08 '20 22:07 BryceBeagle

uhm, let's see if other people have other feedbacks as well :) I don't use enums that much to be honest :)

patrick91 avatar Jul 09 '20 08:07 patrick91

I would go for the name and then in the code you can get the value of the enum or strawberry could convert it back to an enum instance (so you don't get "BLAH" as string, but Enum BLAH.name/BLAH.value)

It's more consistent with how enums are in Strawberry and "pythonic" enough

Or, isn't it a bit confusing to use the value? If you are debugging an enum you can't search for the value you send to the API because it might not be defined in the code but be an auto()?

Also I think you can already use generate_next_value in the Enum without Strawberry supporting it?

marcoacierno avatar Jul 09 '20 12:07 marcoacierno

My idea was to use something along the lines of the following, but using the @strawberry.enum decorator instead of inheritance

class AutoName(Enum):
    def _generate_next_value_(name, start, count, last_values):
        return name

class Ordinal(AutoName):
    NORTH = auto()
    SOUTH = auto()
    EAST = auto()
    WEST = auto()

>>> list(Ordinal)
[<Ordinal.NORTH: 'NORTH'>, <Ordinal.SOUTH: 'SOUTH'>, <Ordinal.EAST: 'EAST'>, <Ordinal.WEST: 'WEST'>]

This would set the value to be the same as the name. (I actually took this example from the _generate_next_value docs)

I'm just afraid other new users will fall for the same trap I did and think that if they assign values, they'll be used instead. By recommending (enforcing?) that users use auto() (that we control with _generate_next_value), I think this confusion can be avoided. We can let the users define their own values if they really want, or we can have them default to match the name.

If you are debugging an enum you can't search for the value you send to the API because it might not be defined in the code but be an auto()?

I don't think this would be an issue if the value=name

Also I think you can already use generate_next_value in the Enum without Strawberry supporting it?

Of course, but I was thinking this could be the default behavior. Anyone that uses generate_next_value is probably already aware of how Strawberry gets the Enum name.

BryceBeagle avatar Jul 09 '20 16:07 BryceBeagle

Related: https://github.com/graphql-python/graphene/pull/1153

We could also have enums like this:

Ordinal = strawberry.enum('NORTH', 'SOUTH')

Ordinal.SOUTH

so they are more inline with GraphQL :)

patrick91 avatar Jul 10 '20 08:07 patrick91

I bring one (perhaps) related question that maybe brings something else to the conversation. Currently i'm trying to define year enum likeso:

class ConferenceYear(Enum):
    2020 = '2020'
    2019 = '2019'

Obviously this doesn't work becuase 2020 and 2019 are not valid identifiers. My workaround was:

class ConferenceYear(Enum):
    _2020 = '2020'
    _2019 = '2010'

But with the current approach 'Value is the enum name' my workaround leaks into the api response and i get _2020 instead of the desired 2020. Perhaps we could define a helper class strawberry.enum_value which allows to override the exposed name and perhaps also adding a description. Something like

class ConferenceYear(Enum):
    _2020 = strawberry.enum_value(name='2020', description='Year 2020')
    _2019 = '2019'

Does it make to have something like that? Is it currently supported by any other way that i don't know of?

Ambro17 avatar Aug 02 '20 13:08 Ambro17

Good idea! I’m not sure about the name, maybe it is better to have strawberry.enum.value, but not 100% sure.

I feel like enums in GraphQL are more like Unions and Literal in python: https://www.python.org/dev/peps/pep-0586/#shortening-unions-of-literals

Didn’t expect unions to be this painful :D

I’ll think about this a bit more, happy to hear more feedbacks

patrick91 avatar Aug 02 '20 13:08 patrick91

Is 2020 a valid enum identifier for GraphQL? I think you would have the same issue looking here: https://spec.graphql.org/June2018/#sec-Names and https://spec.graphql.org/June2018/#sec-Enum-Value

marcoacierno avatar Aug 02 '20 14:08 marcoacierno

Mmm great reference. It seems that even if we provide the custom name option, it wouldn't be able to start with a number.. I think it can provide value nevertheless. For example if i have an enum name called from or with or any name that might clash with python syntax that we would want to avoid

Ambro17 avatar Aug 02 '20 14:08 Ambro17

This issue been added to the v1 milestones, but what I'm having trouble parsing the following from the discussion:

  • What was the conclusion on using the python Enum value vs name as the GraphQL enum name?
  • What feature are we aiming to implement by v1? Is it the strawberry.enum_value that @Ambro17 suggested?

BryceBeagle avatar Sep 22 '20 07:09 BryceBeagle

Is there any way to work around this at the moment?

We're looking to use the enum values for some enums.

jordangarside avatar Apr 08 '21 18:04 jordangarside

Is there any way to work around this at the moment?

We're looking to use the enum values for some enums.

going to make a new release with support for this soon :)

patrick91 avatar Apr 08 '21 22:04 patrick91

This issue been added to the v1 milestones, but what I'm having trouble parsing the following from the discussion:

  • What was the conclusion on using the python Enum value vs name as the GraphQL enum name?
  • What feature are we aiming to implement by v1? Is it the strawberry.enum_value that @Ambro17 suggested?

I totally missed this. We haven't made a decision on this, let's discuss it now (as we started a chat on discord).

We have some options for handling this[1]

A. add a parameter

@strawberry.enum(name_from_value=True)
class UserType(Enum):
    A = "a"
    B = "b"
    BLAH = "c"

benefits of this: similar to what we have, easy to implement and works with mypy (and also does autocompletion in various editors) cons: confusing

B. new API that allows to pass the enum values:

Ordinal = strawberry.enum(name="Ordinal", values=('NORTH', 'SOUTH'))

Ordinal.SOUTH

benefits: it works like GraphQL enums cons: it's a breaking change, we need to update the mypy plugin, autocompletion won't work, name becomes mandatory

with this option we can still support python enums (if needed) with:

@strawberry.enum.from_python
class UserType(Enum):
    A = "a"
    B = "b"
    BLAH = "c"

[1] btw no matter what we decided we should keep the current api for backwards compatibility

patrick91 avatar Apr 09 '21 09:04 patrick91

This issue been added to the v1 milestones, but what I'm having trouble parsing the following from the discussion:

  • What was the conclusion on using the python Enum value vs name as the GraphQL enum name?
  • What feature are we aiming to implement by v1? Is it the strawberry.enum_value that @Ambro17 suggested?

I totally missed this. We haven't made a decision on this, let's discuss it now (as we started a chat on discord).

We have some options for handling this[1]

A. add a parameter

@strawberry.enum(name_from_value=True)
class UserType(Enum):
    A = "a"
    B = "b"
    BLAH = "c"

benefits of this: similar to what we have, easy to implement and works with mypy (and also does autocompletion in various editors) cons: confusing

B. new API that allows to pass the enum values:

Ordinal = strawberry.enum(name="Ordinal", values=('NORTH', 'SOUTH'))

Ordinal.SOUTH

benefits: it works like GraphQL enums cons: it's a breaking change, we need to update the mypy plugin, autocompletion won't work, name becomes mandatory

with this option we can still support python enums (if needed) with:

@strawberry.enum.from_python
class UserType(Enum):
    A = "a"
    B = "b"
    BLAH = "c"

[1] btw no matter what we decided we should keep the current api for backwards compatibility

I'm a fan of using #1, as it reduces the number of ways to define an Enum. It also probably would reduce development and maintenance burden especially if the API around strawberry enum's change in the future. Given that strawberry is dataclasses first, it makes more sense to define things as classes IMO.

We can just get around this with better documentation with clear use cases. Thoughts?

peniqliotuv avatar Apr 09 '21 14:04 peniqliotuv

I'm a much bigger fan of the class-oriented declaration style over the function-oriented style. It feels more Pythonic to me, and as @peniqliotuv says, it feels closer to the dataclass approach we've been going for. Much more declarative than imperative.

As far as controlling whether to use the Enum name vs value, can we make that an option on the schema instead of per-Enum? Similar to the configuration for the auto-camelCasing feature we're working on.

BryceBeagle avatar Apr 09 '21 15:04 BryceBeagle

As far as controlling whether to use the Enum name vs value, can we make that an option on the schema instead of per-Enum? Similar to the configuration for the auto-camelCasing feature we're working on.

that's an option, @jordangarside would that work for you?

patrick91 avatar Apr 09 '21 15:04 patrick91

so we could this:

class Config(StrawberryConfig):
   enums_from_values = True

strawberry.Schema(Query, config=Config())

Reminder: if we use the values as names we should do validation, I've seen enums where the values had spaces in it at work, like:

@strawberry.enum
class UserType(Enum):
    A = "a"
    B = "b"
    BLAH = "example value"

patrick91 avatar Apr 09 '21 19:04 patrick91

B. new API that allows to pass the enum values:

Ordinal = strawberry.enum(name="Ordinal", values=('NORTH', 'SOUTH'))

Ordinal.SOUTH

benefits: it works like GraphQL enums cons: it's a breaking change, we need to update the mypy plugin, autocompletion won't work, name becomes mandatory

Good point with the cons here and they are probably enough to mean that we shouldn't go with this approach. It's a shame though because it more closely matches GraphQL. The GraphQL docs define enums as:

enumeration types are a special kind of scalar that is restricted to a particular set of allowed values. [link]

and Python enums are a mapping from members to values. So any kind of conversion is going to loose some information and potentially be confusing. Maybe NamedTuple is a better match (https://docs.python.org/3/library/typing.html#typing.NamedTuple) or actually just a dataclass since we use it everywhere else?

I also prefer the strawberry.enum.from_python API when converting Python enum's since it makes it clear that it's a conversion rather than a straight definition.

Also @patrick91 you're missing out on a great meta API with the config option 😛 :

class Config(StrawberryConfig):
   enum_values = StrawberryConfig.ENUM_MEMBER # or StrawberryConfig.ENUM_VALUE

jkimbo avatar Apr 10 '21 07:04 jkimbo

enumeration types are a special kind of scalar that is restricted to a particular set of allowed values. [link]

and Python enums are a mapping from members to values. So any kind of conversion is going to loose some information and potentially be confusing.

If this is a big deal, we could enforce only using enum.auto() as the values. That way there's only one item for a user to consider.

BryceBeagle avatar Apr 10 '21 07:04 BryceBeagle

enumeration types are a special kind of scalar that is restricted to a particular set of allowed values. [link]

and Python enums are a mapping from members to values. So any kind of conversion is going to loose some information and potentially be confusing.

If this is a big deal, we could enforce only using enum.auto() as the values. That way there's only one item for a user to consider.

True but then you pretty much loose the benefit of using Python enums. Anyway we would have to support a strawberry.enum_value value to support this case: https://github.com/strawberry-graphql/strawberry/issues/370#issuecomment-667676951

jkimbo avatar Apr 10 '21 07:04 jkimbo

FWIW, I do tend to think that typing.Literal (https://docs.python.org/3/library/typing.html#typing.Literal) is the closest starting point in Python's type system to GraphQL enums.

It's also closer to what the TypeScript community is starting to advocate for instead of using their enum construct, because it's more lightweight.

I suppose the downside is losing the ability to have something like DIRECTION.WEST in code, and having to use just WEST

AndrewIngram avatar Apr 13 '21 11:04 AndrewIngram

Literal is nice, and supports annotated, if we'd need to add additional information

from typing import Literal, Annotated

def something(v: str) -> None:
    return None

MODE = Literal['r', 'rb', 'w', Annotated['wb', something('abc')]]

patrick91 avatar Apr 13 '21 11:04 patrick91

thought we still need to do this:

Mode = strawberry.enum(name="Mode", type=Literal['r', 'rb', 'w', Annotated['wb', something('abc')]])

which has the same as issue mentioned before

patrick91 avatar Apr 13 '21 11:04 patrick91

I'm assuming wrapping Literal to have extra powers is back into custom mypy plugins again?

e.g.

Mode = strawberry.enum('r', 'rb', 'w', Annotated['wb', something('abc')])

where all the args get passed to an underlying Literal.

AndrewIngram avatar Apr 13 '21 12:04 AndrewIngram

@AndrewIngram you'd still need to pass name, as we'd need it at runtime. And yes, we can make a plugin for mypy, but reusing Literal might be easier 😊

patrick91 avatar Apr 13 '21 12:04 patrick91

Hey, circling back and was wondering if this is going to be on the roadmap any time soon!

peniqliotuv avatar Aug 10 '21 18:08 peniqliotuv

FWIW, I came across https://github.com/strawberry-graphql/strawberry/commit/537cdfd7cdbd362f8a41999d764c9fcd808c8765#diff-9db4ecb0b6a9104731da4af6d847961cbc88e4085f4f23f4fcfceb1b22e2fd35R37 while debugging that names are used, when I have expected to see the value when serialized.

It seems like the value is meant to be used there, but is not, because the output_value is a str already when it comes there.

Maybe also related to this topic in general: graphql-core has names_as_values (https://github.com/graphql-python/graphql-core/commit/a3e7b487d10b99ef658444df666fad11a76e12d7).

blueyed avatar Apr 28 '22 20:04 blueyed