-
Notifications
You must be signed in to change notification settings - Fork 1
ref: mapper api overhaul #147
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
base: main
Are you sure you want to change the base?
Changes from 9 commits
16d303b
abf6ba0
18ba6e5
5afbf12
912c82e
f972983
9fcd3ee
01276e4
dff45dc
701b55c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,7 +20,7 @@ class TicketOrder: ... | |
| class TicketFilter: ... | ||
|
|
||
|
|
||
| @strawchemy.type(Ticket, include="all", filter_input=TicketFilter, order_by=TicketOrder, override=True) | ||
| @strawchemy.type(Ticket, include="all", filter_input=TicketFilter, order=TicketOrder, override=True) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Search for order_by usage in strawchemy.type() and strawchemy.field() calls
echo "=== Type decorator usage with order_by ==="
rg -nP '@strawchemy\.type\([^)]*order_by\s*=' --type=py
echo -e "\n=== Field usage with order_by ==="
rg -nP 'strawchemy\.field\([^)]*order_by\s*=' --type=py
echo -e "\n=== Type decorator usage with order= (new pattern) ==="
rg -nP '@strawchemy\.type\([^)]*order\s*=' --type=py | head -20
echo -e "\n=== Field usage with order= (new pattern) ==="
rg -nP 'strawchemy\.field\([^)]*order\s*=' --type=py | head -20Repository: gazorby/strawchemy Length of output: 3173 Update field-level API to use The type decorator has been migrated to use
Update all field-level calls to use 🤖 Prompt for AI Agents |
||
| class TicketType: ... | ||
|
|
||
|
|
||
|
|
@@ -55,7 +55,7 @@ class ProjectOrder: ... | |
| class ProjectFilter: ... | ||
|
|
||
|
|
||
| @strawchemy.type(Project, include="all", filter_input=ProjectFilter, order_by=ProjectOrder, override=True) | ||
| @strawchemy.type(Project, include="all", filter_input=ProjectFilter, order=ProjectOrder, override=True) | ||
| class ProjectType: ... | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -10,6 +10,7 @@ | |||||||||||||||||
| from strawchemy.utils.strawberry import default_session_getter | ||||||||||||||||||
|
|
||||||||||||||||||
| if TYPE_CHECKING: | ||||||||||||||||||
| from strawchemy.dto.types import FieldIterable, IncludeFields | ||||||||||||||||||
| from strawchemy.repository.typing import AnySessionGetter, FilterMap | ||||||||||||||||||
| from strawchemy.typing import AnyRepositoryType, SupportedDialect | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -43,14 +44,24 @@ class StrawchemyConfig: | |||||||||||||||||
| """Override default filters with custom filters.""" | ||||||||||||||||||
| execution_options: dict[str, Any] | None = None | ||||||||||||||||||
| """SQLAlchemy execution options for strawberry operations.""" | ||||||||||||||||||
| pagination_default_limit: int = 100 | ||||||||||||||||||
| """Default pagination limit when `pagination=True`.""" | ||||||||||||||||||
| pagination: bool = False | ||||||||||||||||||
| """Enable/disable pagination on list resolvers.""" | ||||||||||||||||||
| default_id_field_name: str = "id" | ||||||||||||||||||
| """Name for primary key fields arguments on primary key resolvers.""" | ||||||||||||||||||
| deterministic_ordering: bool = True | ||||||||||||||||||
| """Force deterministic ordering for list resolvers.""" | ||||||||||||||||||
| include: IncludeFields = "all" | ||||||||||||||||||
| """Globally included fields.""" | ||||||||||||||||||
| exclude: FieldIterable | None = None | ||||||||||||||||||
| """Globally included fields.""" | ||||||||||||||||||
| pagination: IncludeFields | None = None | ||||||||||||||||||
| """Enable/disable pagination on list resolvers.""" | ||||||||||||||||||
| order_by: IncludeFields | None = None | ||||||||||||||||||
| """Enable/disable order by on list resolvers.""" | ||||||||||||||||||
| distinct_on: IncludeFields | None = None | ||||||||||||||||||
| """Enable/disable order by onelist resolvers.""" | ||||||||||||||||||
| pagination_default_limit: int = 100 | ||||||||||||||||||
| """Default pagination limit when `pagination=True`.""" | ||||||||||||||||||
| pagination_default_offset: int = 0 | ||||||||||||||||||
| """Default pagination offset when `pagination=True`.""" | ||||||||||||||||||
|
Comment on lines
+62
to
+65
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inline docstrings reference outdated API. The docstrings on lines 55 and 57 mention 🔎 Proposed fix pagination_default_limit: int = 100
- """Default pagination limit when `pagination=True`."""
+ """Default pagination limit when pagination is enabled."""
pagination_default_offset: int = 0
- """Default pagination offset when `pagination=True`."""
+ """Default pagination offset when pagination is enabled."""📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Comment on lines
+52
to
+65
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix documentation errors in field docstrings. There are two issues in the docstrings:
📝 Proposed fix include: IncludeFields = "all"
"""Globally included fields."""
exclude: FieldIterable | None = None
- """Globally included fields."""
+ """Globally excluded fields."""
pagination: IncludeFields | None = None
"""Enable/disable pagination on list resolvers."""
order_by: IncludeFields | None = None
"""Enable/disable order by on list resolvers."""
distinct_on: IncludeFields | None = None
- """Enable/disable order by onelist resolvers."""
+ """Enable/disable distinct on for list resolvers."""
pagination_default_limit: int = 100
"""Default pagination limit when `pagination=True`."""
pagination_default_offset: int = 0
"""Default pagination offset when `pagination=True`."""🤖 Prompt for AI Agents |
||||||||||||||||||
|
|
||||||||||||||||||
| inspector: SQLAlchemyGraphQLInspector = field(init=False) | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,7 +32,7 @@ | |
| DTOMissing, | ||
| DTOSkip, | ||
| DTOUnset, | ||
| ExcludeFields, | ||
| FieldIterable, | ||
| IncludeFields, | ||
| Purpose, | ||
| PurposeConfig, | ||
|
|
@@ -392,17 +392,26 @@ def should_exclude_field( | |
| explictly_excluded = node.is_root and field.model_field_name in dto_config.exclude | ||
| explicitly_included = node.is_root and field.model_field_name in dto_config.include | ||
|
|
||
| # Exclude fields not present in init if purpose is write | ||
| if dto_config.purpose is Purpose.WRITE and not explicitly_included: | ||
| explictly_excluded = explictly_excluded or not field.init | ||
| globally_excluded = field.model_field_name in dto_config.global_exclude | ||
| globally_included = field.model_field_name in dto_config.global_include | ||
|
|
||
| if dto_config.include == "all" and not explictly_excluded: | ||
| explicitly_included = True | ||
| explicitly_included = globally_included = True | ||
|
|
||
| if dto_config.global_include == "all" and not globally_excluded: | ||
| globally_included = True | ||
|
|
||
| excluded = dto_config.purpose not in field.allowed_purposes | ||
|
|
||
| # Exclude fields not present in init if purpose is write | ||
| if dto_config.purpose is Purpose.WRITE and not (explicitly_included or globally_included): | ||
| excluded = excluded or not field.init | ||
|
|
||
| if node.is_root: | ||
| excluded = excluded or (explictly_excluded or not explicitly_included) | ||
| else: | ||
| excluded = excluded or explictly_excluded | ||
| excluded = excluded or (globally_excluded or not globally_included) | ||
|
|
||
|
Comment on lines
+396
to
+415
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Root DTOs still ignore Lines 396-403 compute the global flags, but the root branch only checks Proposed fix- if dto_config.include == "all" and not explictly_excluded:
+ if dto_config.include == "all" and not (explictly_excluded or globally_excluded):
explicitly_included = globally_included = True
@@
- if node.is_root:
- excluded = excluded or (explictly_excluded or not explicitly_included)
+ if node.is_root:
+ excluded = excluded or (
+ explictly_excluded
+ or globally_excluded
+ or not (explicitly_included or globally_included)
+ )
else:
excluded = excluded or (globally_excluded or not globally_included)🤖 Prompt for AI Agents |
||
| return not has_override and excluded | ||
|
|
||
| def _resolve_basic_type(self, field: DTOFieldDefinition[ModelT, ModelFieldT], dto_config: DTOConfig) -> Any: | ||
|
|
@@ -626,6 +635,7 @@ def factory( | |
| raise_if_no_fields: bool = False, | ||
| tags: set[str] | None = None, | ||
| backend_kwargs: dict[str, Any] | None = None, | ||
| no_cache: bool = False, | ||
| **kwargs: Any, | ||
| ) -> type[DTOBaseT]: | ||
| """Build a Data transfer object (DTO) from an SQAlchemy model.""" | ||
|
|
@@ -640,7 +650,7 @@ def factory( | |
| if dto_config.scope == "global": | ||
| self._scoped_dto_names[self._scoped_cache_key(model, dto_config)] = name | ||
|
|
||
| if (dto := self._dto_cache.get(cache_key)) or (dto := self._dto_cache.get(scoped_cache_key)): | ||
| if not no_cache and ((dto := self._dto_cache.get(cache_key)) or (dto := self._dto_cache.get(scoped_cache_key))): | ||
| return self.backend.copy(dto, name) if node.is_root else dto | ||
|
|
||
| dto = self._factory( | ||
|
|
@@ -685,7 +695,7 @@ def decorator( | |
| model: type[ModelT], | ||
| purpose: Purpose, | ||
| include: IncludeFields | None = None, | ||
| exclude: ExcludeFields | None = None, | ||
| exclude: FieldIterable | None = None, | ||
| partial: bool | None = None, | ||
| type_map: Mapping[Any, Any] | None = None, | ||
| aliases: Mapping[str, str] | None = None, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -129,7 +129,9 @@ class _Key(Generic[T]): | |
| string. | ||
| """ | ||
|
|
||
| separator: str = ":" | ||
| __slots__ = ("_key",) | ||
|
|
||
| separator: ClassVar[str] = ":" | ||
|
Comment on lines
+133
to
+135
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # First, let's look at the _Key class definition around lines 132-134
sed -n '120,160p' src/strawchemy/dto/strawberry.pyRepository: gazorby/strawchemy Length of output: 1545 🏁 Script executed: # Find all subclasses of _Key in the file
rg -n 'class\s+\w+.*\(_Key\)' src/strawchemy/dto/strawberry.py -A 5Repository: gazorby/strawchemy Length of output: 44 🏁 Script executed: # Check the DTOKey class mentioned at line 179
sed -n '175,210p' src/strawchemy/dto/strawberry.pyRepository: gazorby/strawchemy Length of output: 1081 🏁 Script executed: # Search for all classes that inherit from _Key
rg -n '_Key' src/strawchemy/dto/strawberry.py | grep -E '(class|:)'Repository: gazorby/strawchemy Length of output: 124 🏁 Script executed: # Check for dynamic attribute assignments or setattr patterns
rg -n 'setattr|__dict__|self\.\w+\s*=' src/strawchemy/dto/strawberry.py | head -20Repository: gazorby/strawchemy Length of output: 340 🏁 Script executed: # Look at git history to verify if separator was changed from instance to ClassVar
git log -p --all -S 'separator' -- src/strawchemy/dto/strawberry.py | head -100Repository: gazorby/strawchemy Length of output: 44 🏁 Script executed: # Check if DTOKey or its instances need to store additional attributes
rg -n 'DTOKey' src/strawchemy/dto/strawberry.py -B 2 -A 10 | head -50Repository: gazorby/strawchemy Length of output: 2047 🏁 Script executed: # Check if there are any tests that might reveal the actual impact
find . -name '*test*.py' -type f | head -10Repository: gazorby/strawchemy Length of output: 474 🏁 Script executed: # Look for all usages of DTOKey to understand its instantiation patterns
rg -n 'DTOKey' --type py | head -20Repository: gazorby/strawchemy Length of output: 1932 🏁 Script executed: # Verify that DTOKey doesn't add any additional attributes by checking all its methods
sed -n '179,200p' src/strawchemy/dto/strawberry.pyRepository: gazorby/strawchemy Length of output: 738 🏁 Script executed: # Check if DTOKey instances are created in large quantities (memory-critical context)
rg -n 'DTOKey\(' src/strawchemy -c --type py | sort -rnRepository: gazorby/strawchemy Length of output: 140 🏁 Script executed: # Look at the parent _Key's full implementation to understand inheritance model
sed -n '114,176p' src/strawchemy/dto/strawberry.pyRepository: gazorby/strawchemy Length of output: 2205 🏁 Script executed: # Verify if there are any other subclasses of _Key besides DTOKey
rg -n 'class\s+\w+.*\(_Key' src/ --type pyRepository: gazorby/strawchemy Length of output: 129 🏁 Script executed: # Check if separator usage would be different if it were an instance variable vs ClassVar
rg -n 'separator' src/strawchemy/dto/strawberry.py -B 1 -A 1Repository: gazorby/strawchemy Length of output: 479 🏁 Script executed: # Verify Python behavior: check if empty __slots__ in subclass prevents __dict__
python3 << 'EOF'
# Test: Does subclass without __slots__ get a __dict__?
class Parent:
__slots__ = ("x",)
class Child(Parent):
pass
c = Child()
print("Child has __dict__:", hasattr(c, '__dict__'))
print("Parent __slots__:", Parent.__slots__)
print("Child __slots__:", getattr(Child, '__slots__', 'Not defined'))
# With __slots__ = () in child
class ChildWithSlots(Parent):
__slots__ = ()
c2 = ChildWithSlots()
print("\nChildWithSlots has __dict__:", hasattr(c2, '__dict__'))
print("ChildWithSlots __slots__:", ChildWithSlots.__slots__)
EOFRepository: gazorby/strawchemy Length of output: 148 🏁 Script executed: # Simpler test for Python __slots__ behavior
python3 << 'EOF'
class Parent:
__slots__ = ("x",)
class Child(Parent):
pass
try:
c = Child()
c.y = 5
print("Child WITHOUT __slots__ definition can add attributes: True")
except AttributeError:
print("Child WITHOUT __slots__ definition blocked attribute addition")
EOFRepository: gazorby/strawchemy Length of output: 121 🏁 Script executed: # Check how many DTOKey instances are typically created in real usage
rg -n 'DTOKey\(' src/strawchemy/schema/factories/ -B 2 -A 2 | head -30Repository: gazorby/strawchemy Length of output: 2055 🏁 Script executed: # Verify if DTOKey instances are stored in long-lived data structures
rg -n '__strawchemy_field_map__' src/strawchemy -A 1 | head -20Repository: gazorby/strawchemy Length of output: 1639 Fix DTOKey to define The addition of 🤖 Prompt for AI Agents |
||
|
|
||
| def __init__(self, components: Sequence[T | str] | str | None = None) -> None: | ||
| self._key: str = "" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix hyphenation in configuration table.
"snake cased names" should be hyphenated as "snake-cased names" for proper English grammar.
🔎 Proposed fix
📝 Committable suggestion
🧰 Tools
🪛 LanguageTool
[grammar] ~2105-~2105: Use a hyphen to join words.
Context: ... | Automatically convert snake cased names to camel case in GraphQL sch...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents