Bug
When a Source Python program registers an evaluator via repl's set_evaluator() and that evaluator returns Python's None, the Repl tab displays the string "null" instead of "None".
Repro:
from repl import set_evaluator
def my_evaluator(code):
return None
set_evaluator(my_evaluator)
Running any code through the tab's Run button shows null in the output.
Root cause
This isn't really a bug in repl's display logic - the ambiguity is already baked in one layer up, in py-slang's conversion into Conductor's TypedValue system, and repl has no way to recover the lost information.
Conductor's DataType enum (@sourceacademy/conductor) has no slot for "this language's own null/none/nil sentinel" - only DataType.VOID (no return value at all) and DataType.EMPTY_LIST (a genuine empty list). py-slang's pythonToModule conversion maps Python's None onto DataType.EMPTY_LIST as the closest fit:
src/engines/cse/modules.ts:70 (CSE engine)
src/engines/py2js/moduleInterop.ts:112,212 (py2js engine)
py-slang's own code comments acknowledge this is lossy - e.g. src/engines/cse/modules.ts:80-84: "No empty-list special case: EMPTY_LIST is also what Python's None maps to (...), so returning it here for [] would make [] and None collide on the way back out."
By the time an evaluator's return value reaches repl's stringifyReplValue (src/bundles/repl/src/stringify_value.ts:52), it's just { type: DataType.EMPTY_LIST, value: null } - indistinguishable from a real empty list - so it renders 'null'. There is no information left in this repo's repl bundle to recover "this was actually None" from; a purely local fix in modules isn't possible.
Impact radius
This is bigger than a repl display glitch - it's a collision at the Conductor module boundary itself, so it can affect any Conductor-migrated bundle a Python program passes values into or gets values back from, in both directions:
- Python
None passed into a module function, or returned by one, is indistinguishable from a genuine empty list [] on the other side of the boundary. A Python program doing result is None on a module's return value could get a false positive if the module actually meant to return [] (e.g. "no matches" as an empty list vs. "nothing to report" as None), or a false negative the other way around.
- Currently affects any of the 12 already Conductor-migrated bundles when used from Source Python (
repl is just the first place it's been noticed, since it's the one bundle that echoes a student's own return value back verbatim): binary_tree, csg, curve, matrix, midi, pix_n_flix, plotly, repeat, repl, rune, scrabble, sound. Any function signature among these that accepts or returns a list-typed/optional value is a candidate, not just ones that happen to display text.
- Not limited to display/formatting - anywhere downstream Python code branches on
is None vs. checking for an empty list (len(x) == 0, if x:, etc.) against a value that crossed a module boundary is at risk of silently wrong behavior, not just a cosmetic string.
- Will also affect js-slang once it's migrated to Conductor, in its own way (JS
null vs undefined vs [] all needing to stay distinguishable) - the fix needs to be general enough to cover that too, not just patch the Python case.
- Scoped down: this does not affect plain
display(None) in ordinary Source Python code outside of a module boundary crossing - py-slang's own native stringify.ts (used by its display() builtin) already renders None correctly, because that path never goes through the lossy DataType conversion. The bug only shows up where a value is round-tripped through Conductor's TypedValue system.
Why this needs to generalize, not just special-case Python
The fix shouldn't hardcode "if the language is Python, print None" inside repl. Once js-slang is migrated to Conductor, the same code path needs to correctly render JS's own null/undefined too - and the type of fix needs to keep working per-language without repl knowing which language it's talking to.
Suggested design (spans 3 repos - not implementable from modules alone)
@sourceacademy/conductor: add a new DataType.NULL (name TBD) distinct from EMPTY_LIST/VOID, representing a language's own null/none/nil sentinel.
py-slang: map Python's None to DataType.NULL instead of DataType.EMPTY_LIST in both the CSE and py2js conversion layers. (js-slang would eventually do the same for JS's null.)
@sourceacademy/conductor: additionally add an optional stringify(value: TypedValue<DataType>): Promise<string> | string hook to IDataHandler/IInterfacableEvaluator, so each language can render a value using its own existing native stringifier rather than a generic DataType-based fallback. py-slang already has exactly this logic in its own src/utils/stringify.ts (used for its display() builtin, and it already correctly special-cases None there) - it's just never exposed past the Conductor module boundary today.
modules: update stringifyReplValue (src/bundles/repl/src/stringify_value.ts) to call evaluator.stringify?.(result) first, falling back to the current generic DataType-based rendering only when the evaluator doesn't implement the hook.
Step 1+2 alone fixes the []/None collision, but the new DataType.NULL tag still isn't enough by itself to know whether to print "None" or "null" - that's language-specific, which is why step 3 (the stringify() hook) is what actually makes this render correctly per-language without repl hardcoding anything about Python.
Scope note
None of the actual fix can land in this repo alone: @sourceacademy/conductor is consumed here as an npm dependency, and py-slang isn't part of this workspace at all. This issue is to track the problem and proposed design in modules since that's where the symptom is visible (repl's stringification); the DataType/stringify() hook work needs corresponding issues/PRs against conductor and py-slang.
Do not implement yet - filed for discussion/design review first.
Bug
When a Source Python program registers an evaluator via
repl'sset_evaluator()and that evaluator returns Python'sNone, the Repl tab displays the string"null"instead of"None".Repro:
Running any code through the tab's Run button shows
nullin the output.Root cause
This isn't really a bug in
repl's display logic - the ambiguity is already baked in one layer up, in py-slang's conversion into Conductor'sTypedValuesystem, andreplhas no way to recover the lost information.Conductor's
DataTypeenum (@sourceacademy/conductor) has no slot for "this language's own null/none/nil sentinel" - onlyDataType.VOID(no return value at all) andDataType.EMPTY_LIST(a genuine empty list). py-slang'spythonToModuleconversion maps Python'sNoneontoDataType.EMPTY_LISTas the closest fit:src/engines/cse/modules.ts:70(CSE engine)src/engines/py2js/moduleInterop.ts:112,212(py2js engine)py-slang's own code comments acknowledge this is lossy - e.g.
src/engines/cse/modules.ts:80-84: "No empty-list special case: EMPTY_LIST is also what Python's None maps to (...), so returning it here for[]would make[]andNonecollide on the way back out."By the time an evaluator's return value reaches
repl'sstringifyReplValue(src/bundles/repl/src/stringify_value.ts:52), it's just{ type: DataType.EMPTY_LIST, value: null }- indistinguishable from a real empty list - so it renders'null'. There is no information left in this repo'sreplbundle to recover "this was actuallyNone" from; a purely local fix inmodulesisn't possible.Impact radius
This is bigger than a
repldisplay glitch - it's a collision at the Conductor module boundary itself, so it can affect any Conductor-migrated bundle a Python program passes values into or gets values back from, in both directions:Nonepassed into a module function, or returned by one, is indistinguishable from a genuine empty list[]on the other side of the boundary. A Python program doingresult is Noneon a module's return value could get a false positive if the module actually meant to return[](e.g. "no matches" as an empty list vs. "nothing to report" asNone), or a false negative the other way around.replis just the first place it's been noticed, since it's the one bundle that echoes a student's own return value back verbatim):binary_tree,csg,curve,matrix,midi,pix_n_flix,plotly,repeat,repl,rune,scrabble,sound. Any function signature among these that accepts or returns a list-typed/optional value is a candidate, not just ones that happen to display text.is Nonevs. checking for an empty list (len(x) == 0,if x:, etc.) against a value that crossed a module boundary is at risk of silently wrong behavior, not just a cosmetic string.nullvsundefinedvs[]all needing to stay distinguishable) - the fix needs to be general enough to cover that too, not just patch the Python case.display(None)in ordinary Source Python code outside of a module boundary crossing - py-slang's own nativestringify.ts(used by itsdisplay()builtin) already rendersNonecorrectly, because that path never goes through the lossyDataTypeconversion. The bug only shows up where a value is round-tripped through Conductor'sTypedValuesystem.Why this needs to generalize, not just special-case Python
The fix shouldn't hardcode "if the language is Python, print None" inside
repl. Once js-slang is migrated to Conductor, the same code path needs to correctly render JS's ownnull/undefinedtoo - and the type of fix needs to keep working per-language withoutreplknowing which language it's talking to.Suggested design (spans 3 repos - not implementable from
modulesalone)@sourceacademy/conductor: add a newDataType.NULL(name TBD) distinct fromEMPTY_LIST/VOID, representing a language's own null/none/nil sentinel.py-slang: map Python'sNonetoDataType.NULLinstead ofDataType.EMPTY_LISTin both the CSE and py2js conversion layers. (js-slang would eventually do the same for JS'snull.)@sourceacademy/conductor: additionally add an optionalstringify(value: TypedValue<DataType>): Promise<string> | stringhook toIDataHandler/IInterfacableEvaluator, so each language can render a value using its own existing native stringifier rather than a genericDataType-based fallback. py-slang already has exactly this logic in its ownsrc/utils/stringify.ts(used for itsdisplay()builtin, and it already correctly special-casesNonethere) - it's just never exposed past the Conductor module boundary today.modules: updatestringifyReplValue(src/bundles/repl/src/stringify_value.ts) to callevaluator.stringify?.(result)first, falling back to the current genericDataType-based rendering only when the evaluator doesn't implement the hook.Step 1+2 alone fixes the
[]/Nonecollision, but the newDataType.NULLtag still isn't enough by itself to know whether to print"None"or"null"- that's language-specific, which is why step 3 (thestringify()hook) is what actually makes this render correctly per-language withoutreplhardcoding anything about Python.Scope note
None of the actual fix can land in this repo alone:
@sourceacademy/conductoris consumed here as an npm dependency, andpy-slangisn't part of this workspace at all. This issue is to track the problem and proposed design inmodulessince that's where the symptom is visible (repl's stringification); theDataType/stringify()hook work needs corresponding issues/PRs againstconductorandpy-slang.Do not implement yet - filed for discussion/design review first.