VH-Lab / VH-Lab/vhlab-library-python
Two scope decisions left over from #6: the OldSpikeImporters lineage, and who builds the Python NewStim
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 0
- Forks
- 1
- Avg merge
- 1h 53m
- Merged PRs (30d)
- 4
Description
#6 ends with two items that are decisions rather than tasks. #8 did the parts that were not blocked on them and deliberately left these; this issue is so they are asked in one place instead of being re-derived by whoever picks up the next port.
What #8 settled, for context
getstimscript— ported asvhlib.StimDecode.getstimscript. The three MATLAB definitions of that name are not in competition: onlyStimulusDecoding/getstimscript.mis a free function taking a directory name, whilevhlab-toolbox-matlab/file/@dirstruct/getstimscript.mand this library'sOldSpikeImporters/@cksdirstruct/getstimscript.mare class methods,GETSTIMSCRIPT(DS, THEDIR). NDI-matlab's two call sites both pass a char path, so path order never enters into it.get_intervals/set_intervals— ported ontoMeasuredDataand as dict/.mat-struct-tolerant module functions.- Two rows of #6's Tier 2 table were wrong:
getparametersis a NewStim method at its only call site, not an@autocluster/@windowdiscriminatormethod; andget_data's call site reaches a data-bearing subclass, not the@measureddatabase-class bounds check. - The top-level bridge's
downstream_requirementsnow lists every NDI-matlab (18ff517) call into this library with its call site and status.
That leaves the two questions below. Both need a human call; neither should be answered by whoever happens to need the function next.
Question 1 — does the OldSpikeImporters / vhintan_* lineage get ported at all?
There is already a standing decision against it, at the area level, in vhlib/vhlib_matlab_python_bridge.yaml:
SpikeDataExtraction(388 files) — not_applicable. "Acquisition and spike extraction are NDI's job on the Python side: reading these formats belongs in NDI-python readers, not in a translation of this folder. Revisit only if a specific rig's format has no NDI reader."
And three NDI-matlab call sites reach into exactly that area:
| call site | function | what it reaches |
|---|---|---|
+ndi/+setup/+conv/+vhlab/importMeasuredDataCells.m:61 |
get_data(cells{j},[t0 t1]) |
a data-bearing @measureddata subclass — @spikedata and the cks* classes |
+ndi/+gui/gui.m:150 |
vhintan_importcells(ud.ds) |
cksmultipleunit, cksmeasureddata, get_data_par, loaddata, vhintan_loadcell |
+ndi/+gui/gui.m:154 |
vhintan_clusternameref(ud.ds,name,ref) |
cluster_spikewaves_gui |
So the area decision and the dependency list disagree, and one of them has to give. Roughly:
- Hold the area decision. These three NDI-matlab entry points are conversion and GUI paths; NDI-python satisfies them with its own readers and its own UI rather than by translating this folder.
get_data,vhintan_importcellsandvhintan_clusternamerefbecomenot_applicablehere, with the reason recorded, and NDI-python#134's siblings are told where to go instead. - Narrow it. Port only the read path needed to open cells that already exist in old experiment files —
@spikedata/cksmultipleunit/cksmeasureddata/loaddata/vhintan_loadcellandget_dataon top of them — and leave acquisition and clustering out. This is the option that makesimportMeasuredDataCellsportable. - Case by case, which in practice means the decision gets made three times by three people.
Worth knowing before choosing: vhintan_clusternameref is not a data function. It drives cluster_spikewaves_gui, an interactive spike-sorting GUI, and writes intan_st_*.txt / intan_ci_*.mat cluster files back into the recording directories. Porting it faithfully is a UI question, and NDI plausibly wants its own sorting UI rather than this one.
Concretely: is option 1 or option 2 right, and if 2, is importMeasuredDataCells the only motivating caller?
Question 2 — who builds the Python NewStim, and does vhlib depend on it?
getstimscript now loads stims.mat and hands back saveScript exactly as the .mat loader produces it, because there is no Python stimscript to turn it into. That boundary is where a whole cluster of this repository stops:
| function | what it needs from NewStim |
|---|---|
getstimscriptparameters |
parameters off a stimscript |
getstimscriptsync |
stimscript + sync record |
mti_from_stimtimes_txt |
takes a stimscript as input |
findtransitionsSGSBL |
STOCHASTICGRIDSTIM / BLINKINGSTIM objects |
check_stimtimes_txt_vs_stims_mat |
walks the stimscript in stims.mat |
stimscriptgraph |
currently raises NotImplementedError for this reason |
repairoverflow_stimtimes_txt |
getDisplayOrder(saveScript); ported but incomplete without it |
NDI-matlab's NewStimStims.m:37 — getparameters(get(ss,i)) — is on the same boundary, and it is the reason ndi.daq.metadatareader.NewStimStims cannot be finished in Python.
The home for this appears to exist already: VH-Lab/vhlab-NewStim-python, whose README scopes it to "interpret data structures and read data files created in vhlab-NewStim-matlab" — exactly what is wanted here — but at a8bdd55 it is still a skeleton (an empty src/vhlab_newstim/__init__.py and a test that asserts the import works).
So: is vhlab-NewStim-python the intended home, and should vhlib take a dependency on it once it can read a saveScript? The alternative — a minimal stimscript reader living here — would be quick for one or two functions and wrong for seven, and would duplicate whatever that repository eventually does. If it is the intended home, the seven entries above can say so (not_yet_ported, blocked on a named repository) instead of each saying "blocked on the NewStim object model".
A note on what a reader has to handle: MATLAB writes saveScript as an old-style @stimscript object, which scipy.io.loadmat surfaces as a MATLAB-object wrapper rather than a struct, and savemat cannot write one — so tests for that path need real stims.mat fixtures recorded from MATLAB, not synthesized ones. #8's tests stand a struct in for saveScript for exactly this reason, and they check the directory and error logic only.
Minor, related: a private loader
vhlib.StimDecode.getstimscript and repairoverflow_stimtimes_txt both read .mat files through vlt.file.dirstruct._load_mat_file — the right implementation (scipy for v5/v6/v7, h5py for v7.3), but private by its name. vltMissingFunctions.md now asks vhlab-toolbox-python for a public equivalent. Not blocking anything; recorded so it is not rediscovered twice more.
🤖 Generated with Claude Code
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with vhlib/vhlib_matlab_python_bridge.yaml and the three NDI-matlab call sites listed in the issue, then read the vhlab-NewStim-python README and its skeleton test. Decide whether OldSpikeImporters stays excluded or has a focused read path, and whether vhlib should depend on NewStim Python; done means recording those decisions and updating the affected statuses and destinations.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100