Ensure semimutable vars show public names by Glinte · Pull Request #15 · Glinte/semimutable

Glinte · 2025-08-08T06:18:03Z

Summary

Mirror frozen field values under both public and private keys so vars() exposes public names
Rework class freezing to reserve slots for private fields while retaining a __dict__
Add regression test confirming vars() outputs only public field names

Testing

pytest -q

https://chatgpt.com/codex/tasks/task_e_68958e5b410c83229e0e5acf44cfed7c

codecov-commenter · 2025-08-08T06:18:24Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 82.60870% with 4 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
semimutable/__init__.py	82.60%	3 Missing and 1 partial ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Files with missing lines	Coverage Δ
semimutable/__init__.py	`85.03% <82.60%> (-0.91%)`	⬇️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

This PR ensures that vars() on semimutable dataclass instances exposes only public field names, not the internal private field names used for frozen field storage. The implementation mirrors frozen field values under both public (in __dict__) and private (in slots) keys.

Changes:

Modified FrozenField descriptor to store values in both __dict__ (under public names) and slots (under private names) for classes with __dict__
Updated descriptor's __get__ method to prefer public name from __dict__ when available
Reworked class freezing logic to create slots for private fields while retaining __dict__ for slots=False classes
Added regression test confirming vars() outputs only public field names for slots=False case

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.

File	Description
tests/test_semimutable_dataclass.py	Adds test verifying that `vars()` returns only public field names, not private prefixed names
semimutable/init.py	Updates `FrozenField` descriptor to mirror values in both public (`__dict__`) and private (slots) storage; modifies slots creation logic to support `__dict__` for `slots=False` while providing private field slots

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-17T03:59:01Z

-
-            # Clear existing `__weakref__` descriptor, it belongs to a previous type:
            cls_dict.pop("__weakref__", None)  # gh-102069
+        else:


Setting needs_new_class = True unconditionally for the slots=False case means a new class will always be created even if not strictly necessary for slots. While this simplifies the logic and ensures consistent behavior, consider adding a comment explaining why a new class is required for the slots=False case (presumably to add the private field slots while maintaining dict).

Suggested change

else:

else:

# Even when the original class does not define __slots__, we still need to synthesize

# a new class so that we can introduce our own private frozen-field slots while

# preserving an instance __dict__. This mirrors dataclasses._process_class behavior

# and avoids mutating the original class definition in-place.

Copilot · 2026-01-17T03:59:01Z

+            needs_new_class = True
+            cls_dict["__slots__"] = tuple(FROZEN_PREFIX + field_name for field_name in field_names) + ("__dict__",)
+            for field_name in field_names:
+                cls_dict.pop(field_name, None)
+            cls_dict.pop("__dict__", None)
+            cls_dict.pop("__weakref__", None)


This logic creates __slots__ even when slots=False is specified. The new slots include private frozen field names and __dict__. While this achieves the goal of having both private slots and a __dict__, it may be surprising behavior since users explicitly requested slots=False. Consider documenting this behavior or verifying that this doesn't break expected semantics of slots=False for non-frozen fields.

Suggested change

needs_new_class = True

cls_dict["__slots__"] = tuple(FROZEN_PREFIX + field_name for field_name in field_names) + ("__dict__",)

for field_name in field_names:

cls_dict.pop(field_name, None)

cls_dict.pop("__dict__", None)

cls_dict.pop("__weakref__", None)

# Only synthesize __slots__ for classes without existing slots when slots are enabled.

# When params.slots is False, respect the standard dataclasses semantics by not creating __slots__.

if params.slots:

needs_new_class = True

cls_dict["__slots__"] = tuple(

FROZEN_PREFIX + field_name for field_name in field_names

) + ("__dict__",)

for field_name in field_names:

cls_dict.pop(field_name, None)

cls_dict.pop("__dict__", None)

cls_dict.pop("__weakref__", None)

Copilot · 2026-01-17T03:59:01Z

            )
-
-            # Add our frozen fields to the slots, so they can be used by descriptors.
            cls_dict["__slots__"] += tuple(FROZEN_PREFIX + field_name for field_name in field_names)


Private slots are being created for ALL field names (FROZEN_PREFIX + field_name for field_name in field_names), but only frozen fields actually need them. Non-frozen fields don't use these private slots, making them wasteful. Consider filtering to create private slots only for fields that are actually frozen (those with "frozen" in their metadata), which can be determined by checking f.metadata when iterating over fields(cls).

Copilot · 2026-01-17T03:59:02Z

            cls_dict.pop("__weakref__", None)  # gh-102069
+        else:
+            needs_new_class = True
+            cls_dict["__slots__"] = tuple(FROZEN_PREFIX + field_name for field_name in field_names) + ("__dict__",)


Private slots are being created for ALL field names (FROZEN_PREFIX + field_name for field_name in field_names), but only frozen fields actually need them. Non-frozen fields don't use these private slots, making them wasteful. Consider filtering to create private slots only for fields that are actually frozen (those with "frozen" in their metadata), which can be determined by checking f.metadata when iterating over fields(cls).

Copilot · 2026-01-17T03:59:02Z

+        except Exception:
+            dct = None


The broad except Exception: clause may suppress legitimate errors. Consider catching AttributeError specifically, which is the expected exception when an object doesn't have a __dict__ attribute (e.g., slotted classes). This makes the error handling more precise and avoids masking unexpected exceptions.

Copilot · 2026-01-17T03:59:02Z

+        except Exception:
+            dct = None


The broad except Exception: clause may suppress legitimate errors. Consider catching AttributeError specifically, which is the expected exception when an object doesn't have a __dict__ attribute (e.g., slotted classes). This makes the error handling more precise and avoids masking unexpected exceptions.

Copilot · 2026-01-17T03:59:02Z

 import pytest

 from semimutable import FrozenFieldError, dataclass, field
+from semimutable import FROZEN_PREFIX


FROZEN_PREFIX is imported from semimutable but is not included in the module's __all__ export list. This suggests it's intended as a private implementation detail. Consider either:

Adding FROZEN_PREFIX to __all__ in semimutable/init.py if it's part of the public API

Using a private import approach or accessing it through the internal implementation if it's only needed for testing

Copilot · 2026-01-17T03:59:02Z

+def test_vars_lists_public_field_names_only():
+    @dataclass(slots=False)
+    class Sm:
+        x: int = field(frozen=True)
+        y: int = 0
+
+    sm = Sm(x=1, y=2)
+    d = vars(sm)
+    assert d == {"x": 1, "y": 2}
+    assert FROZEN_PREFIX + "x" not in d


The test only validates the behavior for slots=False. Consider adding a test case for slots=True to ensure that the descriptor logic works correctly for slotted classes, even though slotted classes typically don't have __dict__. This would verify that the fallback to getattr(instance, self._private_name) works as expected.

Mirror frozen field values for vars

886fcee

Glinte added the codex label Aug 8, 2025 — with ChatGPT Codex Connector

Glinte requested a review from Copilot January 17, 2026 03:54

Copilot started reviewing on behalf of Glinte January 17, 2026 03:54 View session

Copilot AI reviewed Jan 17, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure semimutable vars show public names#15

Ensure semimutable vars show public names#15
Glinte wants to merge 1 commit into
mainfrom
codex/update-frozenfield-implementation-and-add-tests

Glinte commented Aug 8, 2025

Uh oh!

codecov-commenter commented Aug 8, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 17, 2026

Uh oh!

Copilot AI Jan 17, 2026

Uh oh!

Copilot AI Jan 17, 2026

Uh oh!

Copilot AI Jan 17, 2026

Uh oh!

Copilot AI Jan 17, 2026

Uh oh!

Copilot AI Jan 17, 2026

Uh oh!

Copilot AI Jan 17, 2026

Uh oh!

Copilot AI Jan 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-        else:
+        else:
+            # Even when the original class does not define __slots__, we still need to synthesize
+            # a new class so that we can introduce our own private frozen-field slots while
+            # preserving an instance __dict__. This mirrors dataclasses._process_class behavior
+            # and avoids mutating the original class definition in-place.

Conversation

Glinte commented Aug 8, 2025

Summary

Testing

Uh oh!

codecov-commenter commented Aug 8, 2025

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants