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

Add support for using raw Python enum types in schema #3639

Merged
merged 16 commits into from
Oct 9, 2024

Conversation

nrbnlulu
Copy link
Member

@nrbnlulu nrbnlulu commented Sep 23, 2024

Description

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Documentation

Issues Fixed or Closed by This PR

fix #3543

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

Summary by Sourcery

Add support for using raw Python enum types in schemas, enabling the use of enums without @strawberry.enum decoration. Fix issue #3543 and update tests to cover the new functionality. Document the changes in RELEASE.md.

New Features:

  • Add support for using raw Python enum types in schemas, allowing enums that are not decorated with @strawberry.enum to be used.

Bug Fixes:

Documentation:

  • Add a new section in RELEASE.md documenting the support for using raw Python enum types in schemas.

Tests:

  • Add tests to verify the default implementation of enum, str enum, and int enum types in schemas.

Copy link
Contributor

sourcery-ai bot commented Sep 23, 2024

Reviewer's Guide by Sourcery

This pull request adds support for using raw Python enum types in Strawberry schemas without requiring the @strawberry.enum decorator. The changes primarily affect the enum handling in the Strawberry library, allowing for more flexible use of enum types from external sources.

File-Level Changes

Change Details Files
Added support for raw Python enum types in schemas
  • Removed the test case that raised an error for non-decorated enums
  • Added new test cases for default enum, StrEnum, and IntEnum implementations
  • Modified the create_enum method to use strawberry_enum for non-decorated enums instead of raising an error
tests/enums/test_enum.py
strawberry/annotation.py
Added release notes for the new feature
  • Created a new RELEASE.md file
  • Described the new functionality and provided an example use case
RELEASE.md

Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • Continue your discussion with Sourcery by replying directly to review comments.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @nrbnlulu - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟡 Testing: 2 issues found
  • 🟢 Complexity: all looks good
  • 🟡 Documentation: 3 issues found

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

tests/enums/test_enum.py Show resolved Hide resolved
Comment on lines 190 to 205
def test_default_int_enum_implementation() -> None:
class Foo(IntEnum):
BAR = 1
BAZ = 2

@strawberry.type
class Query:
@strawberry.field
def foo(self, foo: Foo) -> int:
return foo.value

schema = strawberry.Schema(Query)
res = schema.execute_sync("{ foo(foo: BAR) }")
assert not res.errors
assert res.data
assert res.data["foo"] == 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): New test for default IntEnum implementation

Good test. Consider adding a case to verify that the GraphQL schema represents the enum values as strings, even though they're integers in Python.

def test_default_int_enum_implementation() -> None:
    class Foo(IntEnum):
        BAR = 1
        BAZ = 2

    @strawberry.type
    class Query:
        @strawberry.field
        def foo(self, foo: Foo) -> int:
            return foo.value

    schema = strawberry.Schema(Query)
    res = schema.execute_sync("{ foo(foo: BAR) }")
    assert not res.errors
    assert res.data and res.data["foo"] == 1

    schema_str = str(schema)
    assert '"BAR"' in schema_str and '"BAZ"' in schema_str

RELEASE.md Outdated Show resolved Hide resolved
Comment on lines +9 to +25
```py
# somewhere.py
from enum import Enum


class AnimalKind(Enum):
AXOLOTL, CAPYBARA = range(2)


# gql/animals
from somewhere import AnimalKind


@strawberry.type
class AnimalType:
kind: AnimalKind
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (documentation): Improve the code example for clarity and completeness

Consider adding the missing import strawberry statement, and complete the AnimalKind enum definition. Also, you might want to add a comment to indicate that this is two separate files.

# somewhere.py
from enum import Enum

class AnimalKind(Enum):
    AXOLOTL = 0
    CAPYBARA = 1

# gql/animals.py
import strawberry
from somewhere import AnimalKind

@strawberry.type
class AnimalType:
    kind: AnimalKind

This release adds support for using rwa Python enum types in your schema
(enums that are not decorated with `@strawberry.enum`)

This is useful if you have enum types from other places in your code
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (documentation): Consider expanding on the benefits of this feature

It would be helpful to briefly explain why using raw Python enums in Strawberry schemas is beneficial. This could help users better understand the value of this update.

This is useful if you have enum types from other places in your code
that you want to use in Strawberry without modification. This feature
enhances code reusability, reduces duplication, and simplifies
integration with existing Python libraries or codebases that use enums.

@botberry
Copy link
Member

botberry commented Sep 23, 2024

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:


This release adds support for using raw Python enum types in your schema
(enums that are not decorated with @strawberry.enum)

This is useful if you have enum types from other places in your code
that you want to use in strawberry.
i.e

# somewhere.py
from enum import Enum


class AnimalKind(Enum):
    AXOLOTL, CAPYBARA = range(2)


# gql/animals
from somewhere import AnimalKind


@strawberry.type
class AnimalType:
    kind: AnimalKind

Here's the tweet text:

🆕 Release (next) is out! Thanks to ניר for the PR 👏

Get it here 👉 https://strawberry.rocks/release/(next)

Copy link

codecov bot commented Sep 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.94%. Comparing base (596461b) to head (d0a0a84).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3639      +/-   ##
==========================================
+ Coverage   96.66%   96.94%   +0.27%     
==========================================
  Files         504      503       -1     
  Lines       33448    33457       +9     
  Branches     5600     5602       +2     
==========================================
+ Hits        32333    32434     +101     
+ Misses        881      791      -90     
+ Partials      234      232       -2     

Copy link

codspeed-hq bot commented Sep 23, 2024

CodSpeed Performance Report

Merging #3639 will not alter performance

Comparing nrbnlulu:default-enums (d0a0a84) with main (596461b)

Summary

✅ 15 untouched benchmarks

@nrbnlulu nrbnlulu changed the title Add support for using rwa Python enum types in schemas Add support for using raw Python enum types in schemas Sep 23, 2024
Copy link
Member

@erikwrede erikwrede left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Loving this! Thanks for the PR

strawberry/annotation.py Show resolved Hide resolved
@nrbnlulu nrbnlulu changed the title Add support for using raw Python enum types in schemas Add support for using raw Python enum types in schema Sep 23, 2024
Copy link
Member

@erikwrede erikwrede left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, we should add some docs on this though 😊

Copy link
Member

@patrick91 patrick91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good! I left a couple of comments around the old exception 😊

@@ -13,7 +13,7 @@ example the following code will throw this error:
import strawberry


# note the lack of @strawberry.enum here:
@strawberry.enum here:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nrbnlulu can you fix this? :D

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we can delete this file now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably

@@ -120,25 +120,6 @@ class IceCreamFlavour(Enum):
assert definition.values[2].description is None


@pytest.mark.raises_strawberry_exception(
NotAStrawberryEnumError, match='Enum "IceCreamFlavour" is not a Strawberry enum'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should delete the exception too :)

Copy link
Member

@patrick91 patrick91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed the NotAStrawberryEnumError exception :D

@patrick91 patrick91 mentioned this pull request Oct 8, 2024
Copy link
Member

@erikwrede erikwrede left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

marking as request changes so we dont accidentally merge the test due to previous approval

@patrick91
Copy link
Member

@erikwrede you mean the failing test, right?

@patrick91 patrick91 merged commit 0d880b7 into strawberry-graphql:main Oct 9, 2024
109 checks passed
@erikwrede
Copy link
Member

@patrick91 i just meant the wrong file @nrbnlulu mentioned but looks good now 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Default GraphQL Enums from Python Enums.
4 participants