Repository navigation
ndi.element should reject whitespace in name and type #976
Description
Summary
NDI-python's ndi_element refuses to construct an element whose name or type
contains a space or a tab. ndi.element in MATLAB has no such rule anywhere. We
should add it in MATLAB and mirror Python — and the mock helpers need renaming to
comply, because they are the code that violates it.
The Python check, at src/ndi/element/__init__.py:170:
if " " in name or "\t" in name:
raise ValueError(f"name cannot contain whitespace: '{name}'")
if " " in type or "\t" in type:
raise ValueError(f"type cannot contain whitespace: '{type}'")Why the rule is right, beyond cross-language consistency
ndi.fun.file.pathSafeName turns whitespace into _ (line 38-39), and
ndi.fun.file.elementDirectoryName uses it to build the per-element working
folder for probe export and spike-sorter import. So two distinct elements at
the same reference:
| element name | elementstring() |
directory name |
|---|---|---|
mock stimulator |
mock stimulator | 1 |
mock_stimulator_-_1 |
mock_stimulator |
mock_stimulator | 1 |
mock_stimulator_-_1 |
collide on the same directory. ndi.fun.probe.export.oneProbe,
ndi.fun.probe.import.kilosort.probe and ndi.fun.probe.import.kiasort.probe
would share one working folder between two elements. Banning whitespace at
construction removes that class of collision at the source rather than making
the sanitizer injective, which it cannot be.
(Note the ambiguity argument is not about elementstring() parsing: its
separator is ' | ', so an interior space is not ambiguous. The directory
collision is the concrete hazard.)
Proposed change
Validate in ndi.element's construct-from-arguments path only — the branch
that reaches the assignments at src/ndi/+ndi/element.m:143 and :145 with a
caller-supplied name and type.
Do not validate on the load-from-document path. Python does not: the
constructor returns at element/__init__.py:166 before the check, so an element
that was stored with a space in its name still reads back. Adding the check where
it also guards the document path would make existing stored data unreadable.
Keeping the two paths asymmetric is deliberate and worth a comment in the code,
because it looks like an oversight otherwise.
What needs editing to comply
Only one file actually violates the rule:
src/ndi/+ndi/+mock/+fun/subject_stimulator_neuron.m
- line 48 —
ndi.element.timeseries(S,'mock stimulator',refNum,'stimulator',[],0,subdoc_id) - line 51 —
ndi.element.timeseries(S,'mock spikes',refNum,'spikes',[],0,subdoc_id) - line 181 —
elementNames = {'mock stimulator','mock spikes'}; - lines 19 and 23 — the doc comments naming both
Suggested: mock_stimulator and mock_spikes, which is what NDI-python already
uses. It exports them as constants so the names are not repeated as literals:
from ndi.mock import MOCK_STIMULATOR_NAME, MOCK_SPIKES_NAMEA grep of the repository finds no other code reference to either string —
stimulus_presentation.m and stimulus_response.m mention "mock stimulator"
only in prose.
Migration note
On disk, nothing moves. mock stimulator and mock_stimulator already produce
the same directory (mock_stimulator_-_1), because the sanitizer collapsed the
space — so any stored mock artefact keeps its folder after the rename. That is
the collision described above, seen from the other side: the two spellings were
never distinguishable on disk in the first place.
What does change is anything that matches the element by name — a
ndi.query('element.name','exact_string','mock stimulator','') in user code, or
a saved document referring to it. Those need the new spelling.
Cross-references
- NDI-python PR when creating more than one openminds document, it only creates the first one #239 hit this: it ported
subject_stimulator_neuronand could
not use MATLAB's names, so it introduced the underscore forms as
MOCK_STIMULATOR_NAME/MOCK_SPIKES_NAME. - NDI-python issue ndi.query('','isa',type,'') finds any document type that contains type, instead of exactly equals #211 (bridge drift burn-down) is where the divergence was
found. The rule is recorded nowhere in the bridge YAML and is pinned by three
assertions intests/test_element.py(lines 47, 52, 499), so it was deliberate
on the Python side at some point but never written down or mirrored.
Activity