Skip to content

🐛 Fix TaggedValue Iterable masquerade - #177

Open
elinscott wants to merge 1 commit into
scinode:mainfrom
elinscott:fix-taggedvalue-iterable-masquerade
Open

elinscott wants to merge 1 commit into
scinode:mainfrom
elinscott:fix-taggedvalue-iterable-masquerade

Conversation

@elinscott

Copy link
Copy Markdown
Collaborator

Problem

TaggedValue (the wrapt.ObjectProxy node-graph wraps a socket's value in) always exposed __iter__, because wrapt.ObjectProxy forwards every dunder at the C level regardless of what it wraps. That made isinstance(TaggedValue(1.5), collections.abc.Iterable) report True for a wrapped scalar, even though actually calling iter(...) on it raised TypeError.

This PR closes the Iterable-masquerade itself, for every wrapped scalar; whether that alone makes a given wrapped value pass clean_value depends on what else that function checks.

In practice a wrapped float/int becomes storable, but a wrapped bool still returns the proxy (bool can't be subclassed, so isinstance(TaggedValue(True), bool) stays False) and a wrapped None still fails validation (value is None doesn't hold for the proxy) — both known residuals this PR does not resolve.

>>> from collections.abc import Iterable
>>> isinstance(TaggedValue(1.5), Iterable)
True          # before this fix
>>> iter(TaggedValue(1.5))
TypeError: ...

Changes

  • TaggedValue.__new__ now dispatches on the wrapped value's actual iterability, returning a _TaggedScalar (which shadows __iter__ with None) for non-iterables and a _TaggedIterable (which inherits the proxy's forwarding __iter__) otherwise. Python's Iterable.__subclasshook__ treats __iter__ = None as "intentionally unset" and answers False.
  • The dispatch is transparent to every other TaggedValue behaviour: arithmetic, equality, the socket= kwarg, copy/deepcopy, and serialization all still forward through the wrapped value exactly as before.

Testing

  • Added TestTaggedValueIterable (tests/test_socket.py): a wrapped scalar (float, int, complex, None, bool) is not Iterable and raises TypeError on iter(), while still proxying arithmetic and equality; a wrapped list/tuple/dict/str/set is Iterable and iterates to the same elements as the unwrapped value; the socket= kwarg survives dispatch on both branches.
  • Negative control (reproduced): reverting the __new__ dispatch while keeping the new tests fails 2 of them (test_scalar_is_not_iterable, test_socket_kwarg_preserved_on_scalar_and_iterable) — confirming the tests exercise the fix, not something already true beforehand.
  • Full suite: 317 passed on this branch (upstream main's suite plus the new tests), no regressions.

wrapt.ObjectProxy exposes __iter__ at the C level, so a scalar
TaggedValue reported isinstance(_, Iterable) as True while iter()
raised TypeError — a contradiction that broke any ABC-based
isinstance check downstream (e.g. AiiDA's clean_value).

TaggedValue.__new__ now dispatches on the wrapped value's actual
iterability, returning a _TaggedScalar (with __iter__ = None, which
the Iterable subclass hook reads as "intentionally unset") for
non-iterables and a _TaggedIterable otherwise.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.29730% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.71%. Comparing base (8d85e61) to head (62c622d).

Files with missing lines Patch % Lines
src/node_graph/socket.py 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #177      +/-   ##
==========================================
+ Coverage   89.68%   89.71%   +0.03%     
==========================================
  Files          81       81              
  Lines        8984     9021      +37     
==========================================
+ Hits         8057     8093      +36     
- Misses        927      928       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@elinscott elinscott changed the title Fix TaggedValue Iterable masquerade 🐛 Fix TaggedValue Iterable masquerade Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant