gcurve(size=…) / gdots(size=…) is silently ignored: the constructed object keeps the default radius and the browser is sent the default size of 6, whatever the caller asked for. Setting .size after construction works, and so does radius= in the constructor — only the constructor's size= is lost.
from vpython import *
g = graph()
d = gdots(graph=g, size=8, color=color.blue)
print(d.size) # 6 — expected 8
d.size = 8
print(d.size) # 8 — the setter is fine
Cause
gobj.size is a derived property backed by _radius (vpython/vpython.py:2207-2212):
@property
def size(self): return 2*self._radius
@size.setter
def size(self, val):
self._radius = val/2
self.addattr('radius')
But gobj.setup's scalar-argument loop (vpython/vpython.py:2168-2174) writes the private name directly, bypassing the property:
for a, val in args.items():
argsToSend.append(a)
if a == 'graph':
val = val.idx
setattr(self, '_' + a, val) # size=8 -> self._size = 8
self._size is a dead attribute — nothing in the package reads it. The send loop immediately below then reads back through the property:
for a in argsToSend:
...
aval = getattr(self, a) # getattr(self,'size') -> 2*self._radius -> 6
cmd[a] = aval
so size: 6 goes on the wire and _size is never consulted again. Minimal demonstration:
class Fake:
def __init__(self): self._radius = 3
@property
def size(self): return 2*self._radius
@size.setter
def size(self, val): self._radius = val/2
o = Fake()
setattr(o, '_size', 8) # what gobj.setup does
o._size # 8 (dead)
getattr(o, 'size') # 6 <-- what is sent
radius=8 works for the same reason in reverse: _radius is the real backing attribute, so setattr(self, '_radius', 8) lands in the right slot and getattr(self, 'radius') reads it back.
Scope
I swept every @property in the gobj class body (lines 2122-2338) for getters that are not simply return self._<name>. size is the only one, so this looks like a single-attribute bug rather than a pattern — but the mechanism (setattr(self, '_' + a, val) against a derived property) would bite any future attribute defined the same way, which is the part worth guarding.
Not verified beyond gobj: other classes have their own setup methods with the same setattr(self, '_' + a, val) idiom and their own derived size/axis properties (e.g. vpython.py:815, :1205, :1322, :1623), where size and axis interact. Those may be fine — the interaction is handled explicitly there — but they are the same shape.
Suggested fix
In gobj.setup's scalar loop, route through the property when one exists, or special-case the derived name:
for a, val in args.items():
argsToSend.append(a)
if a == 'graph':
val = val.idx
if a == 'size': # derived: backed by _radius, not _size
self._radius = val/2
else:
setattr(self, '_' + a, val)
Setting _radius directly rather than calling the public setter is deliberate: the size setter also calls addattr('radius'), and addattr spins on baseObj.sent (vpython.py:305-308). During construction that is at best redundant — the value is already going out in this constructor's own cmd — and on a single-threaded host (Pyodide/wasm) a spin-wait inside a constructor is a deadlock rather than a delay.
Happy to open a PR. Two questions if you have a preference:
- Special-case
size, or make the loop property-aware in general (if isinstance(getattr(type(self), a, None), property))? The general form guards the mechanism; the special case is smaller and cannot surprise anything else.
- Should the other
setup implementations with derived size/axis get the same audit in the same change, or separately?
How it surfaced
Found while adopting this package to run Web VPython under Pyodide in a Web Worker for trinket. Trinket's existing bridge bypasses gobj.setup entirely (it forwards constructor kwargs straight to the GlowScript object), so gdots(size=8) works there today and stops working when the real package is used — which is how a long-standing bug turned up as an apparent regression. Nothing about the worker or the transport is involved; the wire package carries size: 6 on the notebook path too.
gcurve(size=…)/gdots(size=…)is silently ignored: the constructed object keeps the default radius and the browser is sent the defaultsizeof 6, whatever the caller asked for. Setting.sizeafter construction works, and so doesradius=in the constructor — only the constructor'ssize=is lost.Cause
gobj.sizeis a derived property backed by_radius(vpython/vpython.py:2207-2212):But
gobj.setup's scalar-argument loop (vpython/vpython.py:2168-2174) writes the private name directly, bypassing the property:self._sizeis a dead attribute — nothing in the package reads it. The send loop immediately below then reads back through the property:so
size: 6goes on the wire and_sizeis never consulted again. Minimal demonstration:radius=8works for the same reason in reverse:_radiusis the real backing attribute, sosetattr(self, '_radius', 8)lands in the right slot andgetattr(self, 'radius')reads it back.Scope
I swept every
@propertyin thegobjclass body (lines 2122-2338) for getters that are not simplyreturn self._<name>.sizeis the only one, so this looks like a single-attribute bug rather than a pattern — but the mechanism (setattr(self, '_' + a, val)against a derived property) would bite any future attribute defined the same way, which is the part worth guarding.Not verified beyond
gobj: other classes have their ownsetupmethods with the samesetattr(self, '_' + a, val)idiom and their own derivedsize/axisproperties (e.g.vpython.py:815,:1205,:1322,:1623), where size and axis interact. Those may be fine — the interaction is handled explicitly there — but they are the same shape.Suggested fix
In
gobj.setup's scalar loop, route through the property when one exists, or special-case the derived name:Setting
_radiusdirectly rather than calling the public setter is deliberate: thesizesetter also callsaddattr('radius'), andaddattrspins onbaseObj.sent(vpython.py:305-308). During construction that is at best redundant — the value is already going out in this constructor's owncmd— and on a single-threaded host (Pyodide/wasm) a spin-wait inside a constructor is a deadlock rather than a delay.Happy to open a PR. Two questions if you have a preference:
size, or make the loop property-aware in general (if isinstance(getattr(type(self), a, None), property))? The general form guards the mechanism; the special case is smaller and cannot surprise anything else.setupimplementations with derivedsize/axisget the same audit in the same change, or separately?How it surfaced
Found while adopting this package to run Web VPython under Pyodide in a Web Worker for trinket. Trinket's existing bridge bypasses
gobj.setupentirely (it forwards constructor kwargs straight to the GlowScript object), sogdots(size=8)works there today and stops working when the real package is used — which is how a long-standing bug turned up as an apparent regression. Nothing about the worker or the transport is involved; the wire package carriessize: 6on the notebook path too.