Skip to content

Commit 7ca5cbb

Browse files
committed
fix: Comments and typos
1 parent 2818834 commit 7ca5cbb

6 files changed

Lines changed: 6 additions & 48 deletions

File tree

.cursorrules

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@ Never use bare `except:`. Log at appropriate levels (DEBUG, INFO, WARNING, ERROR
6161
**Project-Specific:**
6262
- Logger: `logging.getLogger("serializer")` with WARN level by default
6363
- Custom exception: `IsNotSerializable` for unserializable types
64-
- Iterable serialization swallows `IsNotSerializable` exceptions (see FIXME in code)
6564
- Use debug logging for serialization flow: `logger.debug("Serialize key:%s type:%s", ...)`
6665

6766
## Code Practices
@@ -133,9 +132,7 @@ Review all AI-generated code—especially complex logic and security-sensitive c
133132
## Known Issues & Workarounds
134133

135134
**Known Issues:**
136-
1. Iterable serialization swallows `IsNotSerializable` exceptions (see FIXME in `serialize_iter`)
137-
2. Schema checks can be optimized: TODO comments about skipping checks when not greedy
138-
3. Custom serializers cannot access format or tzinfo (documented limitation in README)
135+
1. Custom serializers cannot access format or tzinfo (documented limitation in README)
139136

140137
**Common Pitfalls:**
141138
- One-element tuples must have trailing comma: `serialize_only = ('field',)` not `('field')`

docs/ARCHITECTURE.md

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -385,21 +385,14 @@ graph TD
385385
- Strict: Explicit control, security (don't leak fields)
386386
- `serialize_only` switches to strict mode
387387

388-
### 6. Swallowing Exceptions in Iterables
389-
**Decision**: Catch `IsNotSerializable` in `serialize_iter()` and continue
390-
**Rationale**:
391-
- Allows partial serialization of mixed-type collections
392-
- Prevents one bad item from breaking entire serialization
393-
- **Known Issue**: May hide legitimate errors (see FIXME in code)
394-
395-
### 7. LRU Cache for Field Introspection
388+
### 6. LRU Cache for Field Introspection
396389
**Decision**: Cache `get_serializable_keys()` results
397390
**Rationale**:
398391
- SQLAlchemy inspection is relatively expensive
399392
- Field names don't change at runtime
400393
- Significant performance improvement for repeated serialization
401394

402-
### 8. Options Pattern
395+
### 7. Options Pattern
403396
**Decision**: Use `namedtuple` for serialization options
404397
**Rationale**:
405398
- Immutable configuration

docs/DEVELOPMENT.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,6 @@ print(serializer.schema._tree) # Inspect rule tree
391391
- Cache key is model instance (works because field names don't change)
392392

393393
**Schema optimization**:
394-
- TODO: Skip `is_included()` checks when not greedy (see code comments)
395394
- Consider caching schema lookups for repeated serialization
396395

397396
**Type checking optimization**:

docs/KNOWLEDGE.md

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -298,16 +298,6 @@ The library caches field names using `@functools.lru_cache`:
298298

299299
**Impact**: Significant performance improvement for repeated serialization of same model type.
300300

301-
### Schema Checks
302-
303-
**Current behavior**: `is_included()` is called for every field, even in strict mode.
304-
305-
**Optimization opportunity** (TODO in code):
306-
- Skip `is_included()` checks when not greedy (strict mode)
307-
- In strict mode, only iterate over `schema.keys`
308-
309-
**Impact**: Minor performance improvement for strict mode serialization.
310-
311301
### Type Checking
312302

313303
**Current implementation**: `isinstance(value, types)` with tuple of types.
@@ -525,16 +515,6 @@ class MyModel(Base, SerializerMixin):
525515

526516
**Note**: This is a documented limitation. May be implemented in future.
527517

528-
### Iterable Serialization Swallows Errors
529-
530-
**Symptom**: Some items missing from serialized list, no error
531-
532-
**Cause**: `serialize_iter()` catches `IsNotSerializable` and continues
533-
534-
**Behavior**: By design - allows partial serialization of mixed-type collections
535-
536-
**Workaround**: Check logs for warnings, or serialize items individually to catch errors.
537-
538518
## API Decisions
539519

540520
### Why Mixin Instead of Base Class?
@@ -587,17 +567,6 @@ class MyModel(Base, SerializerMixin):
587567
- Easy to exclude sensitive fields with negative rules
588568
- Can switch to strict mode when needed
589569

590-
### Why Swallow Exceptions in Iterables?
591-
592-
**Decision**: Catch `IsNotSerializable` in `serialize_iter()` and continue
593-
594-
**Rationale**:
595-
- Allows partial serialization of mixed-type collections
596-
- Prevents one bad item from breaking entire serialization
597-
- Common use case: list with mostly serializable items
598-
599-
**Trade-off**: May hide legitimate errors (see FIXME in code)
600-
601570
### Why LRU Cache for Field Introspection?
602571

603572
**Decision**: Cache `get_serializable_keys()` results

sqlalchemy_serializer/serializer.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ def serialize_iter(self, value: Iterable) -> list:
333333
def serialize_dict(self, value: dict) -> dict:
334334
res = {}
335335
for k, v in value.items():
336-
if self.schema.is_included(k): # TODO: Skip check if is NOT greedy
336+
if self.schema.is_included(k):
337337
logger.debug("Serialize key:%s type:%s of dict", k, get_type(v))
338338

339339
result = self.serialize(value=v, key=k)
@@ -352,7 +352,7 @@ def serialize_model(self, value) -> dict:
352352
keys.update(get_serializable_keys(value))
353353

354354
for k in keys:
355-
if self.schema.is_included(key=k): # TODO: Skip check if is NOT greedy
355+
if self.schema.is_included(key=k):
356356
v = getattr(value, k)
357357
logger.debug(
358358
"Serialize key:%s type:%s of model:%s",

tests/test_serializer_serialize_model__function.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ def test_model(get_instance):
1414
@pytest.mark.parametrize(
1515
"only, expected",
1616
[
17-
# FIXME: last `key` ignored and all the rist is returned is it expected?
17+
# FIXME: last `key` ignored and all the rest is returned is it expected?
1818
# (("model.list.key",), {"model": {"list": [{"key": 123}]}}),
1919
(("dict.key",), {"dict": {"key": 123}}),
2020
(("bool",), {"bool": False}),

0 commit comments

Comments
 (0)