Replace EnumDefinition with StrawberryEnum #3999
Conversation
Reviewer's GuideThis PR systematically replaces the legacy EnumDefinition class and related _enum_definition attributes with the new StrawberryEnum construct, updating all core schema conversion paths, type checks, code generation, and tests to reference StrawberryEnum and strawberry_definition, while preserving backward compatibility through deprecation aliases and warnings. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3999 +/- ##
==========================================
+ Coverage 94.40% 94.42% +0.02%
==========================================
Files 536 536
Lines 34950 35002 +52
Branches 1835 1836 +1
==========================================
+ Hits 32994 33051 +57
+ Misses 1660 1656 -4
+ Partials 296 295 -1 🚀 New features to boost your workflow:
|
CodSpeed Performance ReportMerging #3999 will not alter performanceComparing Summary
|
for more information, see https://pre-commit.ci
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `RELEASE.md:3` </location>
<code_context>
+Release type: patch
+
+Resolve TODO's about EnumDefinition by renaming it to StrawberryEnum
</code_context>
<issue_to_address>
**suggestion (typo):** Consider changing "TODO's" to "TODOs" for correct pluralization.
The correct plural is "TODOs" without the apostrophe.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Thanks for adding the Here's a preview of the changelog: This release changes We also expose from enum import Enum
import strawberry
from strawberry.types.enum import StrawberryEnumDefinition, has_enum_definition
@strawberry.enum
class ExampleEnum(Enum):
pass
has_enum_definition(ExampleEnum) # True
# Now you can use ExampleEnum.__strawberry_definition__ to access the enum definitionHere's the tweet text: |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Summary
This PR performs a systematic codebase-wide refactoring that renames the EnumDefinition class to StrawberryEnum and updates all references throughout the Strawberry GraphQL library. The change affects 17 files across core modules, tests, and integrations, resolving existing TODO comments that had explicitly marked this renaming as planned technical debt.
The refactoring touches several key areas of the codebase:
- Core enum handling: The main enum class in
strawberry/types/enum.pyis renamed fromEnumDefinitiontoStrawberryEnum, with the__all__export list updated accordingly - Schema system: Updates to schema converters, name converters, and type checking logic to use the new class name
- Code generation: Query codegen and federation modules updated to properly handle the renamed enum type
- Type system: Updates to argument handling, annotation processing, and printer logic
- Integrations: Pydantic integration updated to work with the new enum class name
- Test suite: Comprehensive updates to test assertions and mock object creation
The change maintains complete backward compatibility since StrawberryEnum provides the identical interface and functionality as EnumDefinition. The renaming aligns with Strawberry's naming conventions where core types are prefixed with 'Strawberry' (similar to StrawberryType, StrawberryUnion, etc.). All functionality remains exactly the same - this is purely a naming standardization effort that removes deprecated references and improves code consistency across the library.
Confidence score: 5/5
- This PR is extremely safe to merge with no risk of breaking functionality
- Score reflects that this is a systematic renaming with identical interfaces and comprehensive coverage across all affected files
- No files require special attention as the changes are mechanical and maintain exact same behavior
19 files reviewed, no comments
…instead of _enum_definition
for more information, see https://pre-commit.ci
… in __strawberry_definition__
for more information, see https://pre-commit.ci
…berryObjectDefinition
for more information, see https://pre-commit.ci
RELEASE.md
Outdated
| @@ -0,0 +1,3 @@ | |||
| Release type: patch | |||
There was a problem hiding this comment.
Would this be a minor? It does more than just fixing, it also changes variable names
There was a problem hiding this comment.
I agree 100% with you, also will add a deprecated info on the release
for more information, see https://pre-commit.ci
…ion to make ruff pre-commit happy
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR successfully completes a systematic refactoring that renames EnumDefinition to StrawberryEnum throughout the codebase, resolving TODOs in strawberry/schema/schema_converter.py at lines 872 and 885.
Key Changes
- Core renaming:
EnumDefinitionclass renamed toStrawberryEnuminstrawberry/types/enum.py - Attribute migration: All enum definitions now use
__strawberry_definition__instead of_enum_definition - Backward compatibility: Old
_enum_definitionattribute maintained viaDeprecatedDescriptorwith warnings - Type alias:
EnumDefinitionpreserved as a deprecated alias toStrawberryEnumfor imports - Comprehensive updates: All type hints, imports, and isinstance checks updated across 28 files
- Updated TODO comments: Removed resolved TODO comments from schema converter
Impact
- Standardizes naming convention to match other Strawberry types (e.g.,
StrawberryObjectDefinition) - Maintains full backward compatibility with existing code
- All tests updated to use new patterns
- Documentation added in RELEASE.md
Confidence Score: 5/5
- This PR is safe to merge with minimal risk
- The refactoring is comprehensive, systematic, and includes excellent backward compatibility. The deprecated descriptor pattern ensures existing code continues working while warning users. All 28 files are consistently updated, tests are thorough, and the changes follow established patterns in the codebase.
- No files require special attention
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| strawberry/types/enum.py | 5/5 | Renamed EnumDefinition to StrawberryEnum, added deprecation support for _enum_definition, and created backward compatibility alias |
| strawberry/schema/schema_converter.py | 5/5 | Updated all references from EnumDefinition to StrawberryEnum and resolved TODO comments |
| strawberry/schema/compat.py | 5/5 | Updated is_enum() function to check for StrawberryEnum using __strawberry_definition__ attribute |
| strawberry/types/base.py | 5/5 | Updated type checking logic to use StrawberryEnum and __strawberry_definition__ instead of _enum_definition |
| strawberry/utils/deprecations.py | 5/5 | Added _ENUM_DEFINITION deprecation message for backward compatibility |
| tests/test_deprecations.py | 5/5 | Added comprehensive tests for _enum_definition deprecation and EnumDefinition alias |
| strawberry/types/arguments.py | 5/5 | Replaced EnumDefinition imports and type checks with StrawberryEnum, updated __strawberry_definition__ usage |
| RELEASE.md | 5/5 | Added release notes documenting the EnumDefinition to StrawberryEnum rename and deprecation |
Sequence Diagram
sequenceDiagram
participant Dev as Developer
participant Enum as @strawberry.enum
participant Process as _process_enum()
participant Def as StrawberryEnum
participant Depr as DeprecatedDescriptor
participant Code as User Code
Dev->>Enum: Decorates Enum class
Enum->>Process: _process_enum(cls)
Process->>Def: Create StrawberryEnum instance
Process->>Code: Set cls.__strawberry_definition__
Process->>Depr: Create DeprecatedDescriptor
Depr->>Code: Inject _enum_definition property
Note over Code,Depr: New pattern (recommended)
Code->>Def: Access via __strawberry_definition__
Def-->>Code: Returns StrawberryEnum
Note over Code,Depr: Old pattern (deprecated)
Code->>Depr: Access via _enum_definition
Depr->>Depr: Emit deprecation warning
Depr-->>Code: Returns same StrawberryEnum
Note over Dev,Code: Type checking in schema conversion
Code->>Code: isinstance(type_, StrawberryEnum)
Code->>Code: type_.__strawberry_definition__
28 files reviewed, no comments
|
Ready for review again @bellini666 |
There was a problem hiding this comment.
This change makes has_object_definition return incorrect results. For now, the type guard is
def has_object_definition(
obj: Any,
) -> TypeGuard[type[WithStrawberryObjectDefinition]]:which is now true for enums, as they fulfill if hasattr(obj, "__strawberry_definition__").
However, StrawberryEnum (formerly EnumDefinition) and StrawberryObjectDefinition don't overlap.
We should fix this problem before merging - ideally without introducing additional checks in all other type guards in this file. The same would go for a rename of _scalar_definition. Either, we decide to rename everything into _strawberry_definition, or keep it fanned out over the different types (scalar, enum, [input-]object).
I don't have a clear preference on how we do this as long as the code is kept clean and avoids additional checks 😊
Thanks for your review @erikwrede ! Do you think this behavior is incorrect now? |
|
@Ckk3 I just realized I must've been on the incorrect branch when checking this locally, just double checked the diff. The tests are great though! |
erikwrede
left a comment
erikwrede
left a comment
There was a problem hiding this comment.
No further change requests from my side 😊
Oh okay, thank you😊! |
patrick91
changed the title
patrick91
force-pushed
the
issue-3998
branch
from
f0f092b to
e1d02c2
Compare
patrick91
force-pushed
the
issue-3998
branch
from
e1d02c2 to
fbe5061
Compare
tests/benchmarks/test_stadium.py
Outdated
| - South Stand: 12,500 seats (50 rows × 250 seats) | ||
| - East Stand: 10,000 seats (40 rows × 250 seats) | ||
| - West Stand: 10,000 seats (40 rows × 250 seats) | ||
| - North Stand: 12,500 seats (50 rows x 250 seats) |
5 participants
Description
Replace
EnumDefinitionwithStrawberryEnumTypes of Changes
Issues Fixed or Closed by This PR
Checklist
Summary by Sourcery
Replace the legacy EnumDefinition with StrawberryEnum throughout the core codebase, ensuring all conversions, codegen paths, and tests are updated to use the new class name.
Enhancements:
Documentation:
Summary by Sourcery
Migrate all enum-related logic to use the new StrawberryEnum class, remove the legacy EnumDefinition, and introduce deprecation aliases and warnings for backward compatibility
Enhancements:
Documentation:
Tests: