Skip to content

Add dataset symmetry tests for artifact generation and verification - #39

Merged
stevevanhooser merged 20 commits into
mainfrom
claude/add-symmetry-tests-python-jM9pu
Mar 17, 2026
Merged

stevevanhooser merged 20 commits into
mainfrom
claude/add-symmetry-tests-python-jM9pu

Conversation

@stevevanhooser

Copy link
Copy Markdown
Contributor

Summary

This PR adds comprehensive symmetry tests for NDI dataset functionality, mirroring MATLAB test suites. The tests verify that Python can generate and read dataset artifacts, including database persistence, document serialization, and file attachments.

Key Changes

  • Artifact Generation (make_artifacts/dataset/test_build_dataset.py):

    • Creates test datasets with ingested sessions containing 5 demoNDI documents
    • Each document includes file attachments for binary I/O testing
    • Exports dataset database, individual JSON documents, and a summary manifest
    • Artifacts are persisted to a shared directory for cross-language verification
  • Artifact Verification (read_artifacts/dataset/test_build_dataset.py):

    • Parameterized tests that verify artifacts from both MATLAB and Python sources
    • Validates dataset identity (ID and reference)
    • Verifies session lists match expected values
    • Confirms all exported JSON documents can be loaded from the dataset database
    • Tests binary file attachment reading and content verification
  • Test Infrastructure:

    • Parameterized source_type fixture to run tests against both matlabArtifacts and pythonArtifacts
    • Graceful skipping when artifact directories don't exist
    • Comprehensive error messages for debugging cross-language compatibility issues

Notable Implementation Details

  • The _dataset_summary() helper captures dataset identity, session lists, document inventories, and class counts for comparison
  • File attachment testing verifies that binary documents can be opened, read, and closed correctly
  • Tests use Query API to search and retrieve documents from the dataset database
  • JSON serialization includes allow_nan=True for compatibility with floating-point values

https://claude.ai/code/session_01HbgrZ69hyXBhiCx9tcAuA6

claude added 20 commits March 16, 2026 16:13
Add makeArtifacts and readArtifacts symmetry tests for the dataset
namespace, mirroring the existing session/buildSession pattern. The
makeArtifacts test creates a Dataset with an ingested DirSession
containing 5 demoNDI documents with file attachments, then exports
the database, JSON documents, and a datasetSummary.json manifest.
The readArtifacts test verifies dataset identity, session list,
document inventory, and binary file content from both Python and
MATLAB artifact sources.

https://claude.ai/code/session_01HbgrZ69hyXBhiCx9tcAuA6
- Fix root cause: DatabaseDriver.find() loads raw JSON from SQLite
  without normalizing single-element depends_on lists that were
  unwrapped for MATLAB compatibility, causing field_search to crash
  when iterating over dict keys instead of list elements.
- Add guard in Calculator.search_for_calculator_docs() to skip
  non-dict entries in depends_on.
- Add independent 'symmetry' job to CI workflow that generates
  artifacts then runs read tests.
- Fix lint errors in examples/integration_demo.py.

https://claude.ai/code/session_01HbgrZ69hyXBhiCx9tcAuA6
Replace the Python-only symmetry job with the correct 3-step
cross-language workflow matching the DID-python pattern:

  1. MATLAB makeArtifacts — install NDI-matlab via matbox, generate
     MATLAB artifacts to tempdir
  2. Python makeArtifacts + readArtifacts — generate Python artifacts,
     then read both MATLAB and Python artifacts
  3. MATLAB readArtifacts — read Python-generated artifacts in MATLAB

All steps run in a single job so temp artifacts persist across steps.
Uses matlab-actions/setup-matlab@v2, ehennestad/matbox-actions, and
checks out VH-Lab/NDI-matlab alongside NDI-python.

https://claude.ai/code/session_01HbgrZ69hyXBhiCx9tcAuA6
Three fixes for cross-language symmetry test failures:

1. Dataset datasetSummary.json: Align Python's makeArtifacts output
   with MATLAB's format (numSessions, references, sessionIds,
   sessionSummaries). Update readArtifacts to parse these keys and
   compare per-session summaries using compareSessionSummary.

2. Dataset binary files: Remove the demoNDI-specific binary file
   test that assumed Python's artifact structure. MATLAB's dataset
   may not use the same document types.

3. Session summary comparison: Add skipFieldsIfOneEmpty parameter
   to compareSessionSummary so that fields like daqSystemNames,
   daqSystemDetails, and probes are tolerated when one side is
   empty — Python cannot reconstruct MATLAB DAQ systems from a
   copied session directory.

https://claude.ai/code/session_01HbgrZ69hyXBhiCx9tcAuA6
…LAB sessions

Root cause: DatabaseDriver.find() loads raw JSON from SQLite without
normalizing the document_class.superclasses field. The MATLAB-compatible
storage layer unwraps single-element lists, so a superclass like
[{"definition": "..."}] becomes {"definition": "..."} (a bare dict).
When doc_isa() iterates superclasses calling .get("definition") on each
element, it gets string keys instead of dicts, causing AttributeError.

Fix: Extend the normalization in find() to re-wrap both depends_on and
superclasses, mirroring sqlitedb._normalize_loaded_props. This fixes
DAQ system reconstruction from MATLAB-created session databases —
daqsystem_load now correctly returns DAQSystem objects with their
readers, navigators, and metadata readers intact.

Revert the skipFieldsIfOneEmpty workaround in compareSessionSummary
since the underlying bug is fixed. Python can and should reconstruct
DAQ systems and probes from MATLAB sessions.

https://claude.ai/code/session_01HbgrZ69hyXBhiCx9tcAuA6
…etry

DatabaseDriver.find() was bypassing DID-python's document loading path
by running raw SQL and parsing JSON blobs directly. This skipped
_normalize_loaded_props(), requiring hand-rolled normalization that was
incomplete (missed superclasses). Replace with DID's search() + get_docs()
so normalization lives in one place.

Fix sessionSummary to report MATLAB-compatible class names using
NDI_DAQREADER_CLASS/NDI_FILENAVIGATOR_CLASS/NDI_DAQSYSTEM_CLASS class
attributes instead of Python's type().__qualname__.

Implement epochnodes() on DAQSystem and FileNavigator, matching MATLAB's
output format: epochtable entries (minus epoch_number) with objectname
and objectclass fields, serialized for cross-language JSON comparison.

https://claude.ai/code/session_01HbgrZ69hyXBhiCx9tcAuA6
Multiple fixes to make Python correctly reconstruct epoch data from
MATLAB session artifacts:

FileNavigator._load_from_document: Handle dict-typed document_properties
(from DB) in addition to attribute-based access. The code was using
getattr() on dicts, silently getting None and defaulting to empty
file parameters.

Parse MATLAB cell array strings (e.g. "{ '#.rhd', '#.ndi' }") as
ordered lists instead of eval'ing to Python sets (which lose order).
This preserves pattern ordering for epoch file grouping and hash
computation.

Handle MATLAB '#' wildcard in file patterns by converting to glob '*'.

Use common filename stem (not full first filename) for epochid and
epochprobemap file naming, matching MATLAB's convention.

Hash raw fileparameters string for filematch_hashstring to produce
the same MD5 as MATLAB.

Implement epochnodes() on DAQSystem and FileNavigator. Implement
_load_epochprobemap_file to read TSV probe map files. Add missing
epochprobemap field to underlying_epochs dict.

Construct DAQSystemMFDAQ (not base DAQSystem) when document says
ndi_daqsystem_class contains "mfdaq". Delegate IntanReader.t0_t1
to SpikeInterfaceReader.

Handle session.id as both method and property via _get_session_id
helper.

https://claude.ai/code/session_01HbgrZ69hyXBhiCx9tcAuA6
Comprehensive instructions for another Claude Code instance to create
NDR-python as a faithful mirror of NDR-matlab, following the same
lead-follow architecture, bridge YAML protocol, and symmetry test
framework used in NDI-python.

https://claude.ai/code/session_01HbgrZ69hyXBhiCx9tcAuA6
Resolve conflicts between class rename PR (#38) and our functional
fixes (epoch discovery, DID API, serialization helpers). Keep both
the new MATLAB-mirror class names and all epoch/DAQ reconstruction
improvements.

https://claude.ai/code/session_01HbgrZ69hyXBhiCx9tcAuA6
Rewrite ndi_daq_reader_mfdaq_intan to use vlt.hardware.intan for header
parsing and direct binary data reading, matching how NDI-matlab uses
vhlab-toolbox-matlab. Add vhlab-toolbox-python as a core dependency.

- Use read_Intan_RHD2000_header from vlt.hardware.intan for header parsing
- Implement native binary data reading for amplifier, aux, ADC, and digital channels
- Convert raw uint16 data to physical units (microvolts, volts)
- Update tests to mock vlt header instead of SpikeInterface adapter
- Add vhlab-toolbox-python to pyproject.toml dependencies

https://claude.ai/code/session_01HbgrZ69hyXBhiCx9tcAuA6
Replace vlt.hardware.intan with ndr.format.intan functions:
- read_Intan_RHD2000_header for header parsing
- read_Intan_RHD2000_datafile for data reading (replaces custom _read_raw_data)
- Intan_RHD2000_blockinfo for total sample calculation in t0_t1

Add NDR-python as a project dependency in pyproject.toml.

https://claude.ai/code/session_01HbgrZ69hyXBhiCx9tcAuA6
Add PascalCase aliases for core classes so they can be imported with
conventional Python naming:
- ndi.document.Document (alias for ndi_document)
- ndi.query.Query (alias for ndi_query)
- ndi.session.dir.DirSession (alias for ndi_session_dir)
- ndi.dataset.Dataset (alias for ndi_dataset_dir)

Fixes ImportError in symmetry make_artifacts tests.

https://claude.ai/code/session_01HbgrZ69hyXBhiCx9tcAuA6
The symmetry tests are already run by test-symmetry.yml, which is the
better version (has concurrency control, workflow_dispatch, cleaner
MATLAB setup). The ci.yml copy was redundant after merging two branches
that each added their own.

https://claude.ai/code/session_01HbgrZ69hyXBhiCx9tcAuA6
…bemap

The class in ndi.epoch.epochprobemap is named ndi_epoch_epochprobemap,
not EpochProbeMap. Import it with an alias to fix the ImportError.

https://claude.ai/code/session_01HbgrZ69hyXBhiCx9tcAuA6
Python was storing "ndi_session_dir" (the Python class name) as the
session_creator, but MATLAB uses feval on this string and needs the
MATLAB package name "ndi.session.dir" instead. Added a mapping from
Python class names to their MATLAB equivalents.

https://claude.ai/code/session_01HbgrZ69hyXBhiCx9tcAuA6
Add ndi.util.classname module with ndi_matlab_classname() and
ndi_python_classname() that implement the documented naming rule:
single _ <-> dot (package separator), double __ <-> literal underscore.

This eliminates:
- The _PYTHON_TO_MATLAB_CREATOR lookup dict in _dataset.py
- Dual-format "ndi.daq.system", "ndi_daq_system" lists in syncrule files
- Hardcoded if/elif chains checking both name formats

All type().__name__ usages that write class names into documents or
artifacts now go through ndi_matlab_classname() so MATLAB can read them.
All comparison sites normalize both sides so either format matches.

https://claude.ai/code/session_01HbgrZ69hyXBhiCx9tcAuA6
Generate the datasetSummary.json from a Dataset opened at the artifact
directory (after jsonDocuments is created) instead of from the original
dataset. This ensures the session summary's "files" field includes both
".ndi" and "jsonDocuments", matching what MATLAB expects when it reads
the Python-generated artifacts.

https://claude.ai/code/session_01HbgrZ69hyXBhiCx9tcAuA6
@stevevanhooser
stevevanhooser merged commit d37efa8 into main Mar 17, 2026
5 checks passed
@stevevanhooser
stevevanhooser deleted the claude/add-symmetry-tests-python-jM9pu branch March 17, 2026 19:42
stevevanhooser pushed a commit that referenced this pull request Aug 30, 2026
#63 and this branch both rewrote the superclass loop. Resolved by taking
both: #63's _definition_to_doc_type (so $NDICALCDOCUMENTPATH resolves, not
just $NDIDOCUMENTPATH) and its warning on an unresolvable superclass, plus
this branch's flattening of the superclass list and union of depends_on.
The unresolvable case now warns AND keeps the entry -- dropping it would
silently shorten the list the schema is validated against, trading one
quiet failure for another.

DID-python #39 and #40 have merged, so both strict xfails XPASSed and are
now plain regression tests: removing a file-bearing document, and the
matlab_tests create-add-search-read-remove round trip.

#40 also cleared apps/calculators/simple_calc and apps/markgarbage/
valid_interval from the known-unvalidatable list, exactly as the gate said
it would, and the local Inf widening in the test is replaced by a call to
did.validate.loads_matlab_json.

Merging #63 also brought its NDIcalc-vis-matlab coverage into view, which
CI sees and a bare checkout does not. Three consequences:

- #63's test_binarydoc_symmetry builds a data/generic_file document, which
  cannot be stored blank: generic_file.json seeds "" for two double fields
  whose schema allows no empty value. Worked around in the fixture, as in
  test_cloud_filehandler, rather than patched in the shared JSON.
- Two NDIcalc definitions fail their own schemas -- hartley_reverse_
  correlation inherits ngrid its schema omits (named by NDI-matlab's own
  sweep in 854a658), and reverse_correlation seeds an out-of-range integer.
  Listed separately from NDI's own tree, since they are absent until
  ndi_install.py runs and a bare checkout must not fail for that.
- stimloopsplitter_calc_schema.json is missing the opening quote of a value
  and is unparseable at NDIcalc's only tag and at its head, so no pin bump
  fixes it. Quarantined, with the test failing if it ever parses.

Verified with the NDIcalc trees installed as CI installs them, against did
8e6f914 (main, carrying both fixes): 1614 passed, 216 skipped, 0 xfail.
black and ruff clean; ndi check 18/18.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018gQMtmEgVKNUhvWakNLPLv
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.

2 participants