WIP: Custom frozen dataclasses#586
Conversation
Reviewer's Guide by SourceryThis pull request introduces a Class diagram for frozen_dataclass decoratorclassDiagram
class frozen_dataclass {
+__init__(cls: type[_T]) type[_T]
}
class dataclasses.dataclass {
+__init__(frozen: bool)
}
class cls {
+__setattr__(name: str, value: t.Any) None
+__delattr__(name: str) None
-_frozen: bool
}
frozen_dataclass --|> dataclasses.dataclass: Uses
frozen_dataclass --|> cls: Decorates
File-Level Changes
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 @@
## master #586 +/- ##
==========================================
+ Coverage 51.29% 52.82% +1.53%
==========================================
Files 25 28 +3
Lines 3488 3669 +181
Branches 686 730 +44
==========================================
+ Hits 1789 1938 +149
- Misses 1404 1426 +22
- Partials 295 305 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
d4dc6e7 to
a490526
Compare
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey @tony - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a note about the performance implications of using
__setattr__and__delattr__. - It would be helpful to include a warning in the documentation about the potential for users to bypass the immutability by modifying the
_frozenattribute.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟡 Testing: 1 issue found
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| width=80, | ||
| height=24, | ||
| expected_error=False, | ||
| ), |
There was a problem hiding this comment.
issue (testing): Edge case: Unfreezing via _frozen attribute
Although tested, the ability to unfreeze by modifying _frozen is a security risk. Consider making _frozen a property with a private setter to prevent this or document this behavior clearly as a known limitation.
d2c13aa to
70e415c
Compare
4353421 to
e18198b
Compare
e77bc7d to
e881b35
Compare
e881b35 to
ba0a3b9
Compare
ab84634 to
5803841
Compare
ba0a3b9 to
0975cfd
Compare
e1f4b1a to
e8dfcd2
Compare
why: Fix type checking errors in the custom frozen_dataclass implementation what: - Added targeted mypy configuration override to disable method-assign errors - Only scoped to libtmux._internal.frozen_dataclass module - Preserves strict type checking across the rest of the codebase refs: Enables inheritance from mutable to immutable dataclasses
…thod-assign` why: Fix type checking errors in the custom frozen_dataclass implementation what: - Added targeted mypy configuration override to disable method-assign errors - Only scoped to libtmux._internal.frozen_dataclass module - Preserves strict type checking across the rest of the codebase refs: Enables inheritance from mutable to immutable dataclasses
…iles from type checking why: The frozen_dataclass_sealable decorator adds attributes and methods dynamically at runtime, which mypy cannot properly analyze in test contexts, resulting in false positive errors. what: - Added mypy override to ignore type errors in tests._internal.test_frozen_dataclass_sealable - Added mypy override to ignore type errors in tests.examples._internal.frozen_dataclass_sealable.test_basic - Preserves strict typing for the implementation code while allowing tests to use dynamic features refs: This addresses the mypy test failures while maintaining type safety for the implementation
…tests from strict checking why: The frozen_dataclass_sealable decorator adds attributes and methods dynamically at runtime which causes false positive errors with static analysis tools. Testing this functionality requires patterns that deliberately violate some rules. what: - Added mypy override to ignore type errors in tests._internal.test_frozen_dataclass_sealable - Added mypy override to ignore type errors in tests.examples._internal.frozen_dataclass_sealable.test_basic - Added per-file ignore for RUF009 (function call in default argument) in test_frozen_dataclass_sealable.py - Preserves strict typing and linting for implementation code while allowing tests to use dynamic features refs: This maintains code quality while acknowledging the inherent limitations of static analysis tools when dealing with Python's dynamic runtime features
why: mypy flags 10 ignores as unused after stub/type updates upstream; re-tag the 6 that still need attr-defined ignores using explicit error code. what: - frozen_dataclass_sealable.py: drop 3 unused, retag seal/is_sealable - tests/_internal/test_frozen_dataclass.py: drop 7 unused, retag 4 (new_field, _internal_cache ×2, _frozen) with explicit attr-defined code
e8dfcd2 to
d18a0fd
Compare
Resolves #588
Summary by Sourcery
Tests:
frozen_dataclassdecorator, covering initialization, immutability, inheritance, edge cases, nested mutability, bidirectional references, and mutation methods.