diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9b7da194..18b6bbe3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -333,7 +333,7 @@ jobs: - name: Import NDI and its sibling packages run: | - "$RUNNER_TEMP/venv/bin/python" -c "import ndi, did, ndr, vlt, vhlib, ndicompress, ndi_ontology" + "$RUNNER_TEMP/venv/bin/python" -c "import ndi, did, ndr, vlt, vhlib, ndicompress, ndi_ontology, ndicalc_vis_matlab" - name: Validate installation # Run from outside the checkout so `import ndi` resolves to the @@ -341,3 +341,25 @@ jobs: working-directory: ${{ runner.temp }} run: | "$RUNNER_TEMP/venv/bin/python" -m ndi check + + - name: Resolve an NDIcalc document definition + # NDIcalc-vis-matlab's definitions are not in this repo: they come + # from the ndicalc-vis-matlab package, found through its `ndi.calc` + # entry point (ndi_common_PathConstants.CalcDoc). Prove a plain + # install finds one, both directly and through DID's + # $NDICALCDOCUMENTPATH placeholder. Run from outside the checkout for + # the same reason as the step above. + working-directory: ${{ runner.temp }} + run: | + "$RUNNER_TEMP/venv/bin/python" - <<'PY' + from did.validate import resolve_definition_path + from ndi.common import ndi_common_PathConstants + from ndi.document import ndi_document + + print("CalcDoc:", ndi_common_PathConstants.CalcDoc) + doc = ndi_document("vision/contrast_tuning") + assert "contrast_tuning" in doc.document_properties, doc.document_properties + path = resolve_definition_path("$NDICALCDOCUMENTPATH/vision/contrast_tuning.json") + assert path, "DID could not resolve $NDICALCDOCUMENTPATH/vision/contrast_tuning.json" + print("vision/contrast_tuning resolved from", path) + PY diff --git a/.gitignore b/.gitignore index 0801ceec..4412d812 100644 --- a/.gitignore +++ b/.gitignore @@ -53,17 +53,3 @@ pythonArtifacts.tar.gz # Generated tutorial HTML (keep locally for review, not in repo) tutorials/*.html *.sqlite - -# Document definitions copied in by ndi_install.py from NDIcalc-vis-matlab -# (install_ndi_common_docs). These are that repo's files, regenerated by every -# install and by CI, so tracking them here would vendor a second copy that -# immediately starts drifting from the dependency that owns them. Scoped to the -# specific subtrees the installer writes -- the rest of database_documents/ and -# schema_documents/ is this repo's own and stays tracked. If NDIcalc-vis-matlab -# ever ships another subtree, add it here too. -src/ndi/ndi_common/database_documents/calc/ -src/ndi/ndi_common/database_documents/neuro/ -src/ndi/ndi_common/database_documents/vision/ -src/ndi/ndi_common/schema_documents/calc/ -src/ndi/ndi_common/schema_documents/neuro/ -src/ndi/ndi_common/schema_documents/vision/ diff --git a/ndi_install.py b/ndi_install.py index e1f592a2..f7ff03d9 100644 --- a/ndi_install.py +++ b/ndi_install.py @@ -41,14 +41,6 @@ "python_path": ".", "description": "VH-Lab data utilities and file formats (not on PyPI)", }, - { - "name": "NDIcalc-vis-matlab", - "repo": "https://github.com/VH-Lab/NDIcalc-vis-matlab.git", - "branch": "main", - "python_path": "", - "ndi_common": True, - "description": "NDI calculator and visualization document definitions", - }, { "name": "Pyraview", "repo": "https://github.com/VH-Lab/Pyraview.git", @@ -370,56 +362,6 @@ def write_pth_file(site_packages: Path, tools_dir: Path) -> Path | None: return None -# --------------------------------------------------------------------------- -# ndi_common document definitions from external dependencies -# --------------------------------------------------------------------------- - - -def install_ndi_common_docs(tools_dir: Path, ndi_root: Path) -> bool: - """Copy ndi_common/{database,schema}_documents from external deps. - - Some dependencies (e.g. NDIcalc-vis-matlab) ship document type - definitions that NDI-python needs at runtime. This copies their - ``ndi_common/database_documents`` and ``ndi_common/schema_documents`` - trees into NDI-python's own ``ndi_common`` folder so they are - discoverable via ``ndi_common_PathConstants.DOCUMENT_PATH``. - """ - import shutil - - ndi_common = ndi_root / "src" / "ndi" / "ndi_common" - ok = True - - for dep in DEPENDENCIES: - if not dep.get("ndi_common"): - continue - dep_dir = tools_dir / dep["name"] - dep_common = dep_dir / "ndi_common" - if not dep_common.is_dir(): - warn(f"{dep['name']}: ndi_common folder not found at {dep_common}") - ok = False - continue - - for sub in ("database_documents", "schema_documents"): - src = dep_common / sub - dst = ndi_common / sub - if not src.is_dir(): - continue - count = 0 - for src_file in src.rglob("*"): - if src_file.is_dir(): - continue - rel = src_file.relative_to(src) - dst_file = dst / rel - dst_file.parent.mkdir(parents=True, exist_ok=True) - shutil.copy2(src_file, dst_file) - count += 1 - detail(f"Copied {count} {sub} files from {dep['name']}") - - success(f"Installed document definitions from {dep['name']}") - - return ok - - # --------------------------------------------------------------------------- # pip installation # --------------------------------------------------------------------------- @@ -516,6 +458,7 @@ def validate() -> tuple[int, int]: ("DID (did.implementations.sqlitedb)", "did.implementations.sqlitedb"), ("DID (did.datastructures)", "did.datastructures"), ("vhlab-toolbox (vlt)", "vlt"), + ("ndicalc-vis-matlab (calculator documents)", "ndicalc_vis_matlab"), ("pyraview (signal pyramids)", "pyraview"), ("numpy", "numpy"), ("networkx", "networkx"), @@ -706,9 +649,6 @@ def main() -> int: fail("Installation failed: see the pip errors above") return 1 - # Copy document definitions from external dependencies - install_ndi_common_docs(tools_dir, ndi_root) - # ── Step 5: Validate ─────────────────────────────────────────────── if args.no_validate: print("\n[5/5] Validation skipped (--no-validate)") diff --git a/pyproject.toml b/pyproject.toml index 86233e02..6cb753bf 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -48,6 +48,12 @@ dependencies = [ # now lives; NDI-matlab pulls the MATLAB half in the same way, through its # requirements.txt. "ndi-ontology @ https://github.com/Waltham-Data-Science/ndi-ontology-python/archive/refs/heads/main.zip", + # NDIcalc-vis-matlab's calculator document definitions and schemas + # (ndi_common/{database,schema}_documents/{calc,neuro,vision}). NDI finds + # them through the package's `ndi.calc` entry point -- see + # ndi.fun.find_calc_directories and ndi_common_PathConstants.CalcDoc -- + # the way NDI-matlab finds sibling NDIcalc*-matlab toolboxes. + "ndicalc-vis-matlab @ https://github.com/VH-Lab/NDIcalc-vis-matlab/archive/refs/heads/main.zip", "numpy>=1.20.0", "networkx>=2.6", "jsonschema>=4.0.0", diff --git a/src/ndi/check.py b/src/ndi/check.py index deeba280..519fe3d8 100644 --- a/src/ndi/check.py +++ b/src/ndi/check.py @@ -82,10 +82,14 @@ def check(name: str, ok: bool, detail: str = "") -> None: definitions = _DIDPathConstants.DEFINITIONS missing = [k for k in _DID_PLACEHOLDERS if k not in definitions] + + def _all_dirs(value) -> bool: + # The calculator placeholders hold a list, as in MATLAB. + values = value if isinstance(value, (list, tuple)) else [value] + return all(Path(str(v)).is_dir() for v in values) + unresolved = [ - k - for k in _DID_PLACEHOLDERS - if k in definitions and not Path(str(definitions[k])).is_dir() + k for k in _DID_PLACEHOLDERS if k in definitions and not _all_dirs(definitions[k]) ] if missing: check( @@ -108,6 +112,36 @@ def check(name: str, ok: bool, detail: str = "") -> None: except Exception as e: check("DID path globals ($NDI...PATH)", False, str(e)) + # Calculator document definitions. They live in separate packages + # (ndicalc-vis-matlab, a runtime dependency) found through the ndi.calc + # entry-point group, so a broken or missing one leaves every calculator + # document type unresolvable while NDI's own types still work. + try: + from ndi.common import ndi_common_PathConstants + + calc_doc = ndi_common_PathConstants.CalcDoc + absent = [str(p) for p in calc_doc if not p.is_dir()] + if not calc_doc: + check( + "NDI calculator documents (ndi.calc)", + False, + "no calculator package found; install ndicalc-vis-matlab", + ) + elif absent: + check( + "NDI calculator documents (ndi.calc)", + False, + f"no such directory: {', '.join(absent)}", + ) + else: + check( + "NDI calculator documents (ndi.calc)", + True, + f"{len(calc_doc)} package(s)", + ) + except Exception as e: + check("NDI calculator documents (ndi.calc)", False, str(e)) + # NDR and NDI-compress are real runtime dependencies that nothing else here # imports, so a missing or broken one used to go unreported until a read. ok, detail = _try_import("ndr") diff --git a/src/ndi/common/__init__.py b/src/ndi/common/__init__.py index 5c97ec47..885a63ab 100644 --- a/src/ndi/common/__init__.py +++ b/src/ndi/common/__init__.py @@ -110,6 +110,40 @@ def SCHEMA_PATH(cls, value: Path | None) -> None: def SCHEMA_PATH(cls) -> None: cls._schema_path = None + # CalcDoc / CalcDocSchema carry MATLAB's names (ndi.common.PathConstants). + # Each is a list -- MATLAB's cell array -- with one entry per installed + # calculator package, in ndi.fun.find_calc_directories order. Setting + # either to a list overrides discovery (tests point it at a tmp tree); + # setting None or deleting it rediscovers on the next read. + + @property + def CalcDoc(cls) -> list[Path]: + if cls._calc_doc is None: + cls._calc_doc = cls._find_calculator_document_definitions("database_documents") + return list(cls._calc_doc) + + @CalcDoc.setter + def CalcDoc(cls, value: list[Path] | None) -> None: + cls._calc_doc = None if value is None else [Path(v) for v in value] + + @CalcDoc.deleter + def CalcDoc(cls) -> None: + cls._calc_doc = None + + @property + def CalcDocSchema(cls) -> list[Path]: + if cls._calc_doc_schema is None: + cls._calc_doc_schema = cls._find_calculator_document_definitions("schema_documents") + return list(cls._calc_doc_schema) + + @CalcDocSchema.setter + def CalcDocSchema(cls, value: list[Path] | None) -> None: + cls._calc_doc_schema = None if value is None else [Path(v) for v in value] + + @CalcDocSchema.deleter + def CalcDocSchema(cls) -> None: + cls._calc_doc_schema = None + class ndi_common_PathConstants(metaclass=_PathConstantsMeta): """NDI path constants for document definitions and schemas. @@ -122,6 +156,10 @@ class ndi_common_PathConstants(metaclass=_PathConstantsMeta): COMMON_FOLDER: Path to ndi_common folder with shared resources. DOCUMENT_PATH: Path to document JSON definitions. SCHEMA_PATH: Path to JSON schema files. + CalcDoc: List of paths to NDI calculator document definitions, one + ``/ndi_common/database_documents`` per installed + calculator package (see ``ndi.fun.find_calc_directories``). + CalcDocSchema: The matching list of ``.../schema_documents`` paths. """ # These can be overridden by environment variables or set programmatically @@ -129,6 +167,25 @@ class ndi_common_PathConstants(metaclass=_PathConstantsMeta): _common_folder: Path | None = None _document_path: Path | None = None _schema_path: Path | None = None + _calc_doc: list[Path] | None = None + _calc_doc_schema: list[Path] | None = None + + @classmethod + def _find_calculator_document_definitions(cls, key: str) -> list[Path]: + """MATLAB's private findCalculatorDocumentDefinitions. + + Appends ``ndi_common/`` to every calculator toolbox directory. + MATLAB memoizes the directory list in a ``persistent`` variable; + here CalcDoc / CalcDocSchema memoize the result instead, so deleting + either property re-runs discovery. + """ + if key not in ("database_documents", "schema_documents"): + raise ValueError(f"key must be 'database_documents' or 'schema_documents', not {key!r}") + # Imported here, not at module level: ndi.common is imported while + # ndi itself is still initialising, and ndi.fun pulls in far more. + from ..fun.utils import find_calc_directories + + return [d / "ndi_common" / key for d in find_calc_directories()] @classmethod def _find_ndi_root(cls) -> Path: @@ -161,6 +218,8 @@ def set_paths( common_folder: Path | None = None, document_path: Path | None = None, schema_path: Path | None = None, + calc_doc: list[Path] | None = None, + calc_doc_schema: list[Path] | None = None, ) -> None: """Set custom paths for NDI resources. @@ -169,6 +228,8 @@ def set_paths( common_folder: Custom path to common folder. document_path: Custom path to document definitions. schema_path: Custom path to schema files. + calc_doc: Custom list of calculator document-definition folders. + calc_doc_schema: Custom list of calculator schema folders. """ if ndi_root is not None: cls._ndi_root = Path(ndi_root) @@ -178,6 +239,10 @@ def set_paths( cls._document_path = Path(document_path) if schema_path is not None: cls._schema_path = Path(schema_path) + if calc_doc is not None: + cls._calc_doc = [Path(p) for p in calc_doc] + if calc_doc_schema is not None: + cls._calc_doc_schema = [Path(p) for p in calc_doc_schema] # Paths moved: re-register them with DID so its resolver follows. updateDIDConstants(force=True) @@ -188,14 +253,13 @@ def set_paths( #: NDI's ``$PATH`` placeholders, in the order MATLAB registers them. #: -#: MATLAB keeps calculator definitions in separate toolboxes and so registers -#: ``$NDICALC*PATH`` as a *list* of directories. Python has no separate -#: calculator packages: ``ndi_install.py`` copies every dependency's -#: ``ndi_common/{database,schema}_documents`` tree into NDI-python's own -#: ``ndi_common`` folder, so the calculator placeholders resolve to the same -#: two directories as the non-calculator ones. Recorded rather than papered -#: over: if Python ever grows out-of-tree calculator packages this mapping is -#: where the extra directories belong. +#: As in MATLAB, the two calculator placeholders hold a *list* of directories +#: -- ``ndi_common_PathConstants.CalcDoc`` / ``CalcDocSchema``, one entry per +#: calculator package found by ``ndi.fun.find_calc_directories`` (installed +#: packages registering an ``ndi.calc`` entry point, such as +#: ndicalc-vis-matlab). DID-python's resolver (``did.validate. +#: resolve_definition_path``) tries each entry of a list value in turn, which +#: is what MATLAB's ``did.document.readjsonfilelocation`` does with a cell. _DID_PLACEHOLDERS = ( "$NDISCHEMAPATH", "$NDIDOCUMENTPATH", @@ -204,19 +268,35 @@ def set_paths( ) -def _did_placeholder_values() -> dict[str, str]: - """Current filesystem target for each NDI ``$PATH`` placeholder.""" - document_path = str(ndi_common_PathConstants.DOCUMENT_PATH) - schema_path = str(ndi_common_PathConstants.SCHEMA_PATH) +def _did_placeholder_values() -> dict[str, str | list[str]]: + """Current filesystem target(s) for each NDI ``$PATH`` placeholder.""" return { - "$NDISCHEMAPATH": schema_path, - "$NDIDOCUMENTPATH": document_path, - "$NDICALCSCHEMAPATH": schema_path, - "$NDICALCDOCUMENTPATH": document_path, + "$NDISCHEMAPATH": str(ndi_common_PathConstants.SCHEMA_PATH), + "$NDIDOCUMENTPATH": str(ndi_common_PathConstants.DOCUMENT_PATH), + "$NDICALCSCHEMAPATH": [str(p) for p in ndi_common_PathConstants.CalcDocSchema], + "$NDICALCDOCUMENTPATH": [str(p) for p in ndi_common_PathConstants.CalcDoc], } -def updateDIDConstants(force: bool = False) -> dict[str, str]: +def _document_search_path() -> list[Path]: + """Every folder an NDI document definition may live in, in search order. + + ``DOCUMENT_PATH`` first, then each entry of ``CalcDoc`` -- the folders + MATLAB's ``$NDIDOCUMENTPATH`` and ``$NDICALCDOCUMENTPATH`` name together. + Python-specific helper; MATLAB reaches the same set through DID's globals. + """ + return [ndi_common_PathConstants.DOCUMENT_PATH, *ndi_common_PathConstants.CalcDoc] + + +def _schema_search_path() -> list[Path]: + """Every folder an NDI validation schema may live in, in search order. + + ``SCHEMA_PATH`` first, then each entry of ``CalcDocSchema``. + """ + return [ndi_common_PathConstants.SCHEMA_PATH, *ndi_common_PathConstants.CalcDocSchema] + + +def updateDIDConstants(force: bool = False) -> dict[str, str | list[str]]: """Register NDI's ``$PATH`` placeholders with DID's ``PathConstants``. MATLAB equivalent: ``ndi.common.PathConstants.updateDIDConstants`` (and the @@ -230,6 +310,9 @@ def updateDIDConstants(force: bool = False) -> dict[str, str]: NDI document raises ``Validation file "$NDISCHEMAPATH/....json" not found``. MATLAB has always registered these four keys; Python never did. + Like MATLAB, an empty value is not registered: with no calculator package + installed, ``$NDICALCDOCUMENTPATH`` / ``$NDICALCSCHEMAPATH`` stay absent. + Args: force: Overwrite entries that are already registered. The default (``False``) mirrors MATLAB, which only fills in keys that are @@ -255,6 +338,11 @@ def updateDIDConstants(force: bool = False) -> dict[str, str]: for key in _DID_PLACEHOLDERS: value = values.get(key) if not value: + # MATLAB never registers an empty value. A forced re-registration + # also drops one NDI registered earlier, so a calculator list that + # has since become empty does not leave DID searching stale paths. + if force: + definitions.pop(key, None) continue if force or key not in definitions: definitions[key] = value diff --git a/src/ndi/common/ndi_matlab_python_bridge.yaml b/src/ndi/common/ndi_matlab_python_bridge.yaml index 363a346f..f7eae4ea 100644 --- a/src/ndi/common/ndi_matlab_python_bridge.yaml +++ b/src/ndi/common/ndi_matlab_python_bridge.yaml @@ -130,6 +130,32 @@ classes: access: "class property (read-only)" decision_log: "Exact match. Python uses pathlib.Path." + - name: CalcDoc + type_matlab: "cell array of char" + type_python: "list[Path]" + access: "class property (overridable; None or del rediscovers)" + decision_log: > + MATLAB name and meaning: one /ndi_common/database_documents + per calculator toolbox found by ndi.fun.find_calc_directories + (MATLAB's private findCalculatorDocumentDefinitions). The toolbox + directories come from installed packages' `ndi.calc` entry points + rather than a sibling-folder scan -- see find_calc_directories in + src/ndi/fun/ndi_matlab_python_bridge.yaml. Resolved lazily and + memoized like the other properties here (MATLAB memoizes the + directory list in a persistent variable), and assignable so tests + can point it at a tmp tree. Registered with DID as + $NDICALCDOCUMENTPATH. Before 2026-10-14 this did not exist and + ndi_install.py copied NDIcalc-vis-matlab's trees into + src/ndi/ndi_common instead, which a pip/uv install never ran. + + - name: CalcDocSchema + type_matlab: "cell array of char" + type_python: "list[Path]" + access: "class property (overridable; None or del rediscovers)" + decision_log: > + As CalcDoc, with ndi_common/schema_documents. Registered with DID + as $NDICALCSCHEMAPATH. + methods: - name: set_paths kind: classmethod @@ -146,6 +172,12 @@ classes: - name: schema_path type_python: "Path | None" default: "None" + - name: calc_doc + type_python: "list[Path] | None" + default: "None" + - name: calc_doc_schema + type_python: "list[Path] | None" + default: "None" output_arguments: [] decision_log: > Python-specific method for programmatic path overrides. @@ -164,7 +196,7 @@ classes: default: "False" output_arguments: - name: registered - type_python: "dict[str, str]" + type_python: "dict[str, str | list[str]]" decision_log: > Registers NDI's $PATH placeholders in did.common.PathConstants.DEFINITIONS, so DID can resolve every @@ -172,15 +204,16 @@ classes: this from the constant-property validator mustUpdateDidGlobals and from the static updateDIDConstants; Python has no property validators, so ndi.common calls it at import time. - Divergence, recorded deliberately: MATLAB registers - $NDICALCDOCUMENTPATH / $NDICALCSCHEMAPATH as a cell array of - calculator-toolbox directories found by ndi.fun.find_calc_directories. - Python has no separate calculator packages -- ndi_install.py copies - each dependency's ndi_common/{database,schema}_documents into - NDI-python's own tree -- so both calc placeholders resolve to the - same two directories as $NDIDOCUMENTPATH / $NDISCHEMAPATH. + As in MATLAB, $NDICALCDOCUMENTPATH / $NDICALCSCHEMAPATH hold a + list -- CalcDoc / CalcDocSchema, one folder per calculator package + -- and DID-python's resolve_definition_path tries each entry of a + list value, as MATLAB's readjsonfilelocation does with a cell. (They + used to alias NDI's own two folders, back when ndi_install.py copied + the calculator trees in; that copy is gone, 2026-10-14.) Like MATLAB, an already-registered key is left alone unless - force=True, and an unresolvable $NDIDOCUMENTPATH raises. + force=True, an empty value is not registered, and an unresolvable + $NDIDOCUMENTPATH raises. Python-only: force=True also drops a key + whose value has become empty, so DID does not keep stale paths. Ported 2026-08-29 because DID-python's add_docs began validating documents against their schemas; before that an unregistered placeholder merely made superclass lookups miss silently. diff --git a/src/ndi/document.py b/src/ndi/document.py index 49d63bb2..04a050a9 100644 --- a/src/ndi/document.py +++ b/src/ndi/document.py @@ -32,7 +32,7 @@ except ImportError: DIDDocument = None -from .common import ndi_common_PathConstants, timestamp +from .common import _document_search_path, ndi_common_PathConstants, timestamp from .ido import ndi_ido #: MATLAB reads this out of ``ndi.common.PathConstants.CommonFolder/config`` @@ -71,11 +71,12 @@ def _definition_to_doc_type(definition: str) -> str: by NDIcalc-vis-matlab. MATLAB registers both as DID globals (ndi.common.PathConstants), so both resolve there. - Here they resolve to the same folder, because ndi_install.py copies - each dependency's ``ndi_common`` tree into NDI-python's own. So every - variable is stripped rather than only ``$NDIDOCUMENTPATH`` -- which is - what previously left ``$NDICALCDOCUMENTPATH/...`` as a literal path, - unfindable, and silently skipped. + Both are followed by a relative path, and read_blank_definition searches + DOCUMENT_PATH and every CalcDoc folder for it -- the union of what the + two placeholders hold -- so every variable is stripped rather than only + ``$NDIDOCUMENTPATH``, which is what once left + ``$NDICALCDOCUMENTPATH/...`` as a literal path, unfindable, and + silently skipped. """ import re @@ -986,10 +987,18 @@ def read_blank_definition(document_type: str) -> dict: Returns: Dictionary with blank document structure. """ - root = ndi_common_PathConstants.DOCUMENT_PATH - json_path = root / f"{document_type}.json" + # DOCUMENT_PATH, then every calculator package's folder (CalcDoc) -- + # the folders $NDIDOCUMENTPATH and $NDICALCDOCUMENTPATH name, which + # is where MATLAB's did.document.readjsonfilelocation looks. + roots = _document_search_path() + json_path = None + for root in roots: + candidate = root / f"{document_type}.json" + if candidate.exists(): + json_path = candidate + break - if not json_path.exists() and "/" not in document_type: + if json_path is None and "/" not in document_type: # Fall back to a recursive search for a BARE class name. # # MATLAB resolves a bare name against the whole document tree, @@ -1002,20 +1011,29 @@ def read_blank_definition(document_type: str) -> dict: # Only for a bare name: a caller who wrote 'data/image' # expects that exact file, and should not silently get a # same-named document from elsewhere in the tree. - matches = sorted(root.rglob(f"{document_type}.json")) + matches = [ + (root, m) + for root in roots + if root.is_dir() + for m in sorted(root.rglob(f"{document_type}.json")) + ] if len(matches) == 1: - json_path = matches[0] + json_path = matches[0][1] elif len(matches) > 1: - rel = ", ".join(str(m.relative_to(root)) for m in matches) + rel = ", ".join(str(m.relative_to(root)) for root, m in matches) + first_root, first = matches[0] raise FileNotFoundError( f"ndi_document type {document_type!r} is ambiguous: {rel}. " - f"Give the path, e.g. {matches[0].relative_to(root).with_suffix('')}." + f"Give the path, e.g. {first.relative_to(first_root).with_suffix('')}." ) - if not json_path.exists(): + if json_path is None: + searched = ", ".join(str(r) for r in roots) raise FileNotFoundError( - f"ndi_document definition not found: {json_path}. " - f"Make sure NDI is properly installed." + f"ndi_document definition not found: {document_type}.json " + f"(searched {searched}). Make sure NDI is properly installed; " + f"calculator document types come from calculator packages " + f"such as ndicalc-vis-matlab." ) with open(json_path) as f: @@ -1049,8 +1067,8 @@ def read_blank_definition(document_type: str) -> dict: # Strip whatever $...PATH/ prefix the definition carries, not # just $NDIDOCUMENTPATH: calculator documents name their - # superclasses through $NDICALCDOCUMENTPATH, and ndi_install.py - # merges every dependency's ndi_common into one tree. + # superclasses through $NDICALCDOCUMENTPATH, and the lookup + # searches NDI's own tree and every CalcDoc folder. sc_type = _definition_to_doc_type(sc_def) try: sc_props = ndi_document.read_blank_definition(sc_type) diff --git a/src/ndi/fun/__init__.py b/src/ndi/fun/__init__.py index df810a8b..335a36f6 100644 --- a/src/ndi/fun/__init__.py +++ b/src/ndi/fun/__init__.py @@ -22,6 +22,7 @@ from .text import parse_text, parseText from .utils import ( channelname2prefixnumber, + find_calc_directories, identifier, name2variable_name, name2variableName, @@ -31,6 +32,7 @@ __all__ = [ "channelname2prefixnumber", + "find_calc_directories", "identifier", "plot_extracellular_spikeshapes", "name2variableName", diff --git a/src/ndi/fun/doc.py b/src/ndi/fun/doc.py index 3d02a09c..76c1de15 100644 --- a/src/ndi/fun/doc.py +++ b/src/ndi/fun/doc.py @@ -17,19 +17,17 @@ def allTypes() -> list[str]: MATLAB equivalent: ndi.fun.doc.allTypes + The paths that are searched are given in + ndi.common.PathConstants.DocumentFolder (``DOCUMENT_PATH`` here) and + ndi.common.PathConstants.CalcDoc. + Returns: Sorted list of document type names. """ - from ndi.common import ndi_common_PathConstants + from ndi.common import _document_search_path types: set[str] = set() - doc_folder = ndi_common_PathConstants.COMMON_FOLDER / "database_documents" - search_paths = [doc_folder] - - # Also check calculator doc paths - calc_path = doc_folder / "apps" / "calculators" - if calc_path.exists(): - search_paths.append(calc_path) + search_paths = _document_search_path() for base in search_paths: if not base.exists(): diff --git a/src/ndi/fun/ndi_matlab_python_bridge.yaml b/src/ndi/fun/ndi_matlab_python_bridge.yaml index 5edc5867..d93ffe8a 100644 --- a/src/ndi/fun/ndi_matlab_python_bridge.yaml +++ b/src/ndi/fun/ndi_matlab_python_bridge.yaml @@ -466,7 +466,7 @@ functions: Examined against NDI-matlab f57144891, the file's last-touching commit. Same shape: scan the document folder and the calculator document paths - for .json files, take each file's stem, skip dotfiles, unique and sort. + (DOCUMENT_PATH and ndi_common_PathConstants.CalcDoc) for .json files, take each file's stem, skip dotfiles, unique and sort. Returns the same 101 type names here. One latent divergence: the port additionally strips a trailing '_schema' from the stem, which MATLAB does not -- today no file under database_documents is named that way, so @@ -3401,20 +3401,34 @@ not_applicable: matlab_path: "+ndi/+fun/find_calc_directories.m" matlab_last_sync_hash: "5b9e08493" status: ported_differently - decision_log: > - Not applicable. Finds NDIcalc*-matlab calculator toolboxes by going - three directories up from ndi.toolboxdir and globbing the siblings -- - a search that only works because MATLAB toolboxes are installed side - by side. Python calculators are found through the ndi.calc package - and the class registry (ndi/class_registry.py), which is an import - question rather than a filesystem one; a third-party calculator - distributed as its own package is discovered by being importable. - - Confirmed against NDI-matlab 5b9e08493, the file's last-touching commit. - Still resolves a base path three directories up from ndi.toolboxdir and - globs the siblings, wrapped in a try/catch that warns and returns {}. - Nothing to port: the Python answer is still importability, not the - filesystem. + python_path: "ndi/fun/utils.py" + input_arguments: [] + output_arguments: + - name: d + type_matlab: "cell array of char" + type_python: "list[Path]" + decision_log: > + Same name, same contract -- returns every calculator toolbox directory, + each one containing ndi_common/{database,schema}_documents, and warns + and returns what it could rather than erroring -- but found a different + way. MATLAB goes three directories up from ndi.toolboxdir and globs + sibling NDIcalc*-matlab folders, which works only because MATLAB + toolboxes are installed side by side. A pip/uv install has no such + layout, so a calculator package declares an entry point in the + `ndi.calc` group whose value returns its toolbox directory + (ndicalc-vis-matlab: `NDIcalc-vis-matlab = ndicalc_vis_matlab:toolbox_dir`), + and this reads importlib.metadata.entry_points(group="ndi.calc"), + sorted by entry-point name. An entry point that fails to load, raises, + or returns a non-directory warns (RuntimeWarning) and is skipped, so a + broken calculator package cannot break `import ndi`. Feeds + ndi_common_PathConstants.CalcDoc / CalcDocSchema, as in MATLAB. + + Ported 2026-10-14, replacing the earlier "importability, not the + filesystem" note: calculator *classes* are still found by import, but + their document definitions are data files that must be located, which + ndi_install.py used to do by copying NDIcalc-vis-matlab's trees into + src/ndi/ndi_common -- something a pip/uv install never runs. + Examined against NDI-matlab 5b9e08493, the file's last-touching commit. - name: plot_extracellular_spikeshapes matlab_path: "+ndi/+fun/plot_extracellular_spikeshapes.m" diff --git a/src/ndi/fun/utils.py b/src/ndi/fun/utils.py index 6bd5f7c8..7afc079f 100644 --- a/src/ndi/fun/utils.py +++ b/src/ndi/fun/utils.py @@ -2,14 +2,25 @@ ndi.fun.utils - General utility functions. MATLAB equivalents: +ndi/+fun/channelname2prefixnumber.m, - name2variableName.m, pseudorandomint.m, timestamp.m + name2variableName.m, pseudorandomint.m, timestamp.m, + find_calc_directories.m """ from __future__ import annotations import random import re +import warnings from datetime import datetime, timezone +from pathlib import Path + +from pydantic import validate_call + +#: Entry-point group through which a calculator package announces itself. +#: Each entry point's value is a zero-argument callable returning the +#: package's toolbox directory -- the directory that CONTAINS ``ndi_common/``, +#: just as each directory MATLAB's find_calc_directories returns does. +CALC_ENTRY_POINT_GROUP = "ndi.calc" def channelname2prefixnumber(channelname: str) -> tuple[str, int]: @@ -139,3 +150,72 @@ def identifier(obj) -> str | None: """ value = getattr(obj, "id", None) return value() if callable(value) else value + + +@validate_call +def find_calc_directories() -> list[Path]: + """Find all NDI calculator toolbox directories. + + MATLAB equivalent: ndi.fun.find_calc_directories + + D = ndi.fun.find_calc_directories() + + This function finds installed NDI calculator toolboxes. Each directory + returned contains an ``ndi_common`` folder whose ``database_documents`` + and ``schema_documents`` trees hold the calculator's document + definitions and schemas. + + Returns: + list[Path]: The full path to each calculator directory found. If none + is installed, an empty list. + + Python-specific Notes: + MATLAB scans the folder three levels above ``ndi.toolboxdir`` for + sibling ``NDIcalc*-matlab`` directories, which works because MATLAB + toolboxes are installed side by side. A pip/uv install has no such + layout, so a calculator package instead registers an entry point in + the ``ndi.calc`` group (:data:`CALC_ENTRY_POINT_GROUP`) whose value is + a callable returning its toolbox directory -- for NDIcalc-vis-matlab, + ``ndicalc_vis_matlab:toolbox_dir``. Results are ordered by entry-point + name so the search order is stable. + + Like MATLAB, a failure is a warning and not an error: an entry point + that cannot be loaded, raises, or returns something that is not a + directory is reported with a ``RuntimeWarning`` and skipped, so one + broken calculator package cannot stop ``import ndi``. + """ + from importlib.metadata import entry_points + + try: + found = sorted(entry_points(group=CALC_ENTRY_POINT_GROUP), key=lambda ep: ep.name) + except Exception as exc: # pragma: no cover - a broken metadata store + warnings.warn( + f"An error occurred while trying to find calculator directories: {exc!r}", + RuntimeWarning, + stacklevel=2, + ) + return [] + + d: list[Path] = [] + for ep in found: + try: + directory = Path(ep.load()()) + except Exception as exc: + warnings.warn( + f"Calculator entry point {ep.name!r} ({ep.value}) failed and was " + f"skipped: {exc!r}", + RuntimeWarning, + stacklevel=2, + ) + continue + if not directory.is_dir(): + warnings.warn( + f"Calculator entry point {ep.name!r} ({ep.value}) returned " + f"{str(directory)!r}, which is not a directory; skipped.", + RuntimeWarning, + stacklevel=2, + ) + continue + if directory not in d: + d.append(directory) + return d diff --git a/src/ndi/validate.py b/src/ndi/validate.py index fff79bc9..922f4d3f 100644 --- a/src/ndi/validate.py +++ b/src/ndi/validate.py @@ -17,7 +17,7 @@ from pathlib import Path from typing import TYPE_CHECKING, Any -from .common import ndi_common_PathConstants +from .common import _schema_search_path if TYPE_CHECKING: from .document import ndi_document @@ -122,7 +122,9 @@ class SchemaLoadError(Exception): def _load_schema(schema_name: str) -> dict | None: """Load a schema JSON file by name. - Searches in ndi_common_PathConstants.SCHEMA_PATH for ``{name}_schema.json``. + Searches ndi_common_PathConstants.SCHEMA_PATH, then every folder in + ndi_common_PathConstants.CalcDocSchema (calculator packages), for + ``{name}_schema.json``. Handles subdirectory paths (e.g. 'apps/calculations/simple_calc'). Args: @@ -138,19 +140,29 @@ def _load_schema(schema_name: str) -> dict | None: return _schema_cache[schema_name] try: - schema_path = ndi_common_PathConstants.SCHEMA_PATH + schema_paths = _schema_search_path() except (ValueError, AttributeError): return None - # Try direct path first filename = f"{schema_name}_schema.json" - full_path = schema_path / filename + full_path = None + # Try the direct path in each folder first + for schema_path in schema_paths: + if (schema_path / filename).exists(): + full_path = schema_path / filename + break - if not full_path.exists(): + if full_path is None: # Try searching subdirectories - for candidate in schema_path.rglob(f'{schema_name.split("/")[-1]}_schema.json'): - full_path = candidate - break + basename = f'{schema_name.split("/")[-1]}_schema.json' + for schema_path in schema_paths: + if not schema_path.is_dir(): + continue + for candidate in schema_path.rglob(basename): + full_path = candidate + break + if full_path is not None: + break else: return None diff --git a/tests/bridge_property_allowlist.yaml b/tests/bridge_property_allowlist.yaml index 694fb68d..2177c6c7 100644 --- a/tests/bridge_property_allowlist.yaml +++ b/tests/bridge_property_allowlist.yaml @@ -134,12 +134,6 @@ not_exposed: - python_class: ndi_common_PathConstants property: Preferences reason: "Lazily resolved. See ndi_common_PathConstants.RootFolder." - - python_class: ndi_common_PathConstants - property: CalcDoc - reason: "Lazily resolved. See ndi_common_PathConstants.RootFolder." - - python_class: ndi_common_PathConstants - property: CalcDocSchema - reason: "Lazily resolved. See ndi_common_PathConstants.RootFolder." # ------------------------------------------------------------------------- # ndi.cloud.sync diff --git a/tests/test_calc_doc_discovery.py b/tests/test_calc_doc_discovery.py new file mode 100644 index 00000000..85d14207 --- /dev/null +++ b/tests/test_calc_doc_discovery.py @@ -0,0 +1,252 @@ +"""Calculator document definitions are found in installed packages. + +MATLAB counterpart: ``ndi.fun.find_calc_directories`` and the ``CalcDoc`` / +``CalcDocSchema`` constants of ``ndi.common.PathConstants``. MATLAB scans for +sibling ``NDIcalc*-matlab`` toolbox folders; NDI-python asks every package +registering an ``ndi.calc`` entry point for its toolbox directory, and appends +``ndi_common/database_documents`` (resp. ``schema_documents``) to each. + +Every test here builds a FAKE calculator package in ``tmp_path`` -- a module +plus a ``.dist-info`` carrying the entry point -- so none depends on the +network or on ndicalc-vis-matlab being installed. The last class is the +exception: it checks the real package, and skips when it is absent. +""" + +from __future__ import annotations + +import json +import sys +import textwrap +import uuid +import warnings +from pathlib import Path + +import pytest + +from ndi.common import ndi_common_PathConstants, updateDIDConstants +from ndi.fun import find_calc_directories + + +def _write_json(path: Path, data: dict) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(data)) + + +def _make_fake_calc_package(base: Path, body: str, *, with_docs: bool = True) -> tuple[str, Path]: + """A calculator package that registers ``ndi.calc`` and nothing else. + + ``body`` is the source of the module's ``toolbox_dir``. Returns the + entry-point name and the toolbox directory the module would report. + """ + module = f"fake_ndicalc_{uuid.uuid4().hex[:8]}" + ep_name = f"NDIcalc-fake-{module[-8:]}" + toolbox = base / "toolbox" + + (base / f"{module}.py").write_text( + "from pathlib import Path\n" f"TOOLBOX = Path({str(toolbox)!r})\n\n" + textwrap.dedent(body) + ) + dist_info = base / f"{module}-0.0.1.dist-info" + dist_info.mkdir() + (dist_info / "METADATA").write_text( + f"Metadata-Version: 2.1\nName: {module.replace('_', '-')}\nVersion: 0.0.1\n" + ) + (dist_info / "entry_points.txt").write_text(f"[ndi.calc]\n{ep_name} = {module}:toolbox_dir\n") + + if with_docs: + docs = toolbox / "ndi_common" / "database_documents" + schemas = toolbox / "ndi_common" / "schema_documents" + _write_json( + docs / "fakecalc" / "fake_calc_result.json", + { + "document_class": { + "definition": "$NDICALCDOCUMENTPATH/fakecalc/fake_calc_result.json", + "validation": "$NDICALCSCHEMAPATH/fakecalc/fake_calc_result_schema.json", + "class_name": "fake_calc_result", + "property_list_name": "fake_calc_result", + "class_version": 1, + "superclasses": [{"definition": "$NDIDOCUMENTPATH/base.json"}], + }, + "fake_calc_result": {"value": 0}, + }, + ) + _write_json( + schemas / "fakecalc" / "fake_calc_result_schema.json", + {"classname": "fake_calc_result", "superclasses": ["base"], "properties": []}, + ) + return ep_name, toolbox + + +@pytest.fixture +def calc_path(monkeypatch, tmp_path): + """Put ``tmp_path`` on sys.path and make discovery re-run before and after.""" + from ndi.validate import _schema_cache + + monkeypatch.syspath_prepend(str(tmp_path)) + del ndi_common_PathConstants.CalcDoc + del ndi_common_PathConstants.CalcDocSchema + yield tmp_path + # monkeypatch undoes sys.path after this teardown runs, so drop the cache + # now and rediscover once the fake package is gone. + _schema_cache.pop("fake_calc_result", None) + monkeypatch.undo() + for name in [m for m in sys.modules if m.startswith("fake_ndicalc_")]: + del sys.modules[name] + del ndi_common_PathConstants.CalcDoc + del ndi_common_PathConstants.CalcDocSchema + updateDIDConstants(force=True) + + +GOOD = """ +def toolbox_dir(): + return TOOLBOX +""" + + +class TestFindCalcDirectories: + def test_an_entry_point_contributes_its_toolbox_dir(self, calc_path): + _name, toolbox = _make_fake_calc_package(calc_path, GOOD) + assert toolbox in find_calc_directories() + + def test_a_raising_entry_point_warns_and_is_skipped(self, calc_path): + name, toolbox = _make_fake_calc_package( + calc_path, + """ + def toolbox_dir(): + raise FileNotFoundError("ndi_common is missing") + """, + ) + with pytest.warns(RuntimeWarning, match=name): + found = find_calc_directories() + assert toolbox not in found + + def test_an_unimportable_entry_point_warns_and_is_skipped(self, calc_path): + name, toolbox = _make_fake_calc_package(calc_path, "raise ImportError('broken')\n") + with pytest.warns(RuntimeWarning, match=name): + found = find_calc_directories() + assert toolbox not in found + + def test_a_non_directory_warns_and_is_skipped(self, calc_path): + name, toolbox = _make_fake_calc_package(calc_path, GOOD, with_docs=False) + with pytest.warns(RuntimeWarning, match="not a directory"): + found = find_calc_directories() + assert toolbox not in found + + def test_a_broken_entry_point_does_not_break_calcdoc(self, calc_path): + _make_fake_calc_package(calc_path, "raise ImportError('broken')\n") + with warnings.catch_warnings(): + warnings.simplefilter("ignore", RuntimeWarning) + assert isinstance(ndi_common_PathConstants.CalcDoc, list) + + +class TestCalcDocConstants: + def test_calcdoc_appends_ndi_common_subfolders(self, calc_path): + _name, toolbox = _make_fake_calc_package(calc_path, GOOD) + assert toolbox / "ndi_common" / "database_documents" in ndi_common_PathConstants.CalcDoc + assert toolbox / "ndi_common" / "schema_documents" in ndi_common_PathConstants.CalcDocSchema + + def test_calcdoc_can_be_overridden_and_reset(self, calc_path, tmp_path): + override = [tmp_path / "elsewhere"] + ndi_common_PathConstants.CalcDoc = override + assert ndi_common_PathConstants.CalcDoc == override + ndi_common_PathConstants.CalcDoc = None + assert ndi_common_PathConstants.CalcDoc != override + + def test_did_placeholders_hold_the_lists(self, calc_path): + from did.common import PathConstants + + _name, toolbox = _make_fake_calc_package(calc_path, GOOD) + updateDIDConstants(force=True) + value = PathConstants.DEFINITIONS["$NDICALCDOCUMENTPATH"] + assert isinstance(value, list) + assert str(toolbox / "ndi_common" / "database_documents") in value + + def test_an_empty_list_is_not_registered(self, calc_path): + """MATLAB leaves an empty CalcDoc unregistered; so does this.""" + from did.common import PathConstants + + ndi_common_PathConstants.CalcDoc = [] + ndi_common_PathConstants.CalcDocSchema = [] + updateDIDConstants(force=True) + assert "$NDICALCDOCUMENTPATH" not in PathConstants.DEFINITIONS + assert "$NDICALCSCHEMAPATH" not in PathConstants.DEFINITIONS + + +class TestLookupsSearchCalcDoc: + def test_document_type_resolves_from_the_package(self, calc_path): + from ndi.document import ndi_document + + _make_fake_calc_package(calc_path, GOOD) + d = ndi_document("fakecalc/fake_calc_result") + assert "fake_calc_result" in d.document_properties + assert "base" in d.document_properties + + def test_bare_name_resolves_from_the_package(self, calc_path): + from ndi.document import ndi_document + + _make_fake_calc_package(calc_path, GOOD) + assert ndi_document("fake_calc_result").doc_class() == "fake_calc_result" + + def test_did_resolves_the_calc_placeholder(self, calc_path): + from did.validate import resolve_definition_path + + _name, toolbox = _make_fake_calc_package(calc_path, GOOD) + updateDIDConstants(force=True) + path = resolve_definition_path("$NDICALCSCHEMAPATH/fakecalc/fake_calc_result_schema.json") + assert Path(path) == ( + toolbox + / "ndi_common" + / "schema_documents" + / "fakecalc" + / "fake_calc_result_schema.json" + ) + + def test_schema_is_found_by_ndi_validate(self, calc_path): + from ndi.validate import _load_schema + + _make_fake_calc_package(calc_path, GOOD) + schema = _load_schema("fake_calc_result") + assert schema is not None + assert schema["classname"] == "fake_calc_result" + + def test_alltypes_lists_calc_types(self, calc_path): + from ndi.fun.doc import allTypes + + _make_fake_calc_package(calc_path, GOOD) + assert "fake_calc_result" in allTypes() + + +# --------------------------------------------------------------------------- +# The real package +# --------------------------------------------------------------------------- + + +def _ndicalc_vis_matlab_installed() -> bool: + try: + import ndicalc_vis_matlab # noqa: F401 + except ImportError: + return False + return True + + +@pytest.mark.skipif( + not _ndicalc_vis_matlab_installed(), + reason="ndicalc-vis-matlab is not installed", +) +class TestNDIcalcVisMatlab: + def test_it_is_discovered(self): + import ndicalc_vis_matlab + + assert Path(ndicalc_vis_matlab.toolbox_dir()) in find_calc_directories() + + def test_contrast_tuning_resolves(self): + from ndi.document import ndi_document + + d = ndi_document("vision/contrast_tuning") + assert "contrast_tuning" in d.document_properties + assert "base" in d.document_properties + + def test_contrast_tuning_schema_resolves_through_did(self): + from did.validate import get_document_schema + + schema = get_document_schema("$NDICALCSCHEMAPATH/vision/contrast_tuning_schema.json") + assert schema["classname"] == "contrast_tuning" diff --git a/tests/test_did_integration.py b/tests/test_did_integration.py index e840d0dc..bcab2d5d 100644 --- a/tests/test_did_integration.py +++ b/tests/test_did_integration.py @@ -32,6 +32,24 @@ # $PATH placeholder registration # --------------------------------------------------------------------------- +#: ndicalc-vis-matlab is a runtime dependency, so CI always has a calculator +#: package; `python -m ndi check` fails without one. A local environment +#: built without it still runs everything else here. +needs_calc_package = pytest.mark.skipif( + not ndi_common_PathConstants.CalcDoc, + reason="no calculator package (ndicalc-vis-matlab) is installed", +) + + +def _expected_placeholders(): + """All four, less the calculator two when there is nothing to register. + + MATLAB leaves an empty CalcDoc / CalcDocSchema unregistered too. + """ + if ndi_common_PathConstants.CalcDoc: + return _DID_PLACEHOLDERS + return ("$NDISCHEMAPATH", "$NDIDOCUMENTPATH") + class TestDIDPathConstants: """ndi.common registers NDI's placeholders with did.common.PathConstants.""" @@ -39,15 +57,19 @@ class TestDIDPathConstants: def test_all_placeholders_registered_on_import(self): from did.common import PathConstants - for key in _DID_PLACEHOLDERS: + for key in _expected_placeholders(): assert key in PathConstants.DEFINITIONS, f"{key} not registered with DID" def test_placeholders_resolve_to_real_directories(self): from did.common import PathConstants - for key in _DID_PLACEHOLDERS: - target = Path(str(PathConstants.DEFINITIONS[key])) - assert target.is_dir(), f"{key} -> {target} is not a directory" + for key in _expected_placeholders(): + value = PathConstants.DEFINITIONS[key] + # The calculator placeholders hold a list, as MATLAB's cell does. + targets = value if isinstance(value, list) else [value] + assert targets, f"{key} is registered with no directories" + for target in map(Path, map(str, targets)): + assert target.is_dir(), f"{key} -> {target} is not a directory" def test_document_and_schema_placeholders_point_where_ndi_keeps_them(self): from did.common import PathConstants @@ -60,18 +82,33 @@ def test_document_and_schema_placeholders_point_where_ndi_keeps_them(self): str(ndi_common_PathConstants.SCHEMA_PATH) ) - def test_calc_placeholders_share_the_main_tree(self): - """ndi_install.py merges dependency ndi_common trees into NDI's own. + @needs_calc_package + def test_calc_placeholders_hold_the_calc_lists(self): + """$NDICALC*PATH are lists of calculator folders, as in MATLAB. - MATLAB keeps calculator toolboxes separate and registers a list of - directories; Python has one tree, so the calc placeholders resolve to - the same two directories. Pinned so the divergence is deliberate. + MATLAB registers ndi.common.PathConstants.CalcDoc / CalcDocSchema -- + cell arrays, one entry per calculator toolbox -- and so does this. + They used to alias NDI's own two folders, because ndi_install.py + copied the calculator trees into NDI-python's; nothing is copied now. """ from did.common import PathConstants definitions = PathConstants.DEFINITIONS - assert definitions["$NDICALCDOCUMENTPATH"] == definitions["$NDIDOCUMENTPATH"] - assert definitions["$NDICALCSCHEMAPATH"] == definitions["$NDISCHEMAPATH"] + assert definitions["$NDICALCDOCUMENTPATH"] == [ + str(p) for p in ndi_common_PathConstants.CalcDoc + ] + assert definitions["$NDICALCSCHEMAPATH"] == [ + str(p) for p in ndi_common_PathConstants.CalcDocSchema + ] + + @needs_calc_package + def test_did_resolves_a_calc_definition_through_its_placeholder(self): + """DID walks a list value, so a calculator superclass resolves.""" + from did.validate import resolve_definition_path + + path = resolve_definition_path("$NDICALCDOCUMENTPATH/vision/contrast_tuning.json") + assert path is not None + assert Path(path).name == "contrast_tuning.json" def test_registration_does_not_clobber_an_existing_entry(self): """MATLAB only fills in keys that are absent; so does this.""" @@ -194,10 +231,10 @@ def test_a_document_that_lies_about_its_class_is_rejected(self): "session_in_a_dataset": "DID:Database:ValidationFieldDouble", } -#: The same, for definitions ndi_install.py copies in from NDIcalc-vis-matlab. -#: Kept separate because they are absent on a bare checkout, so their existence -#: is not asserted -- only their behaviour when they are installed. Both were -#: named by NDI-matlab's own sweep (854a658) before this repo could see them. +#: The same, for definitions the ndicalc-vis-matlab package supplies (found +#: through ndi_common_PathConstants.CalcDoc). Kept separate because they belong +#: to that repository, not this one. Both were named by NDI-matlab's own sweep +#: (854a658) before this repo could see them. KNOWN_UNVALIDATABLE_FROM_DEPENDENCIES = { # Inherits ngrid, which its schema omits. "neuro/hartley_reverse_correlation": "DID:Database:ValidationSuperClasses", @@ -236,8 +273,23 @@ def _loads_as_matlab_reads(text): return loads_matlab_json(text) +def _ndi_common_roots(): + """NDI's own ndi_common folder, then each calculator package's.""" + roots = [Path(str(ndi_common_PathConstants.COMMON_FOLDER))] + for folder in ndi_common_PathConstants.CalcDoc: + if folder.parent not in roots: + roots.append(folder.parent) + return roots + + def _bundled_document_types(): - root = Path(str(ndi_common_PathConstants.DOCUMENT_PATH)) + roots = [Path(str(ndi_common_PathConstants.DOCUMENT_PATH))] + roots += list(ndi_common_PathConstants.CalcDoc) + for root in roots: + yield from _document_types_under(root) + + +def _document_types_under(root): for path in sorted(root.rglob("*.json")): try: raw = _loads_as_matlab_reads(path.read_text()) @@ -327,14 +379,15 @@ def test_every_bundled_json_parses(self): Python's stricter parser that had to catch up, which it now does in ``did.validate.loads_matlab_json``. - Files the dependency trees supply are scanned too, so this only checks - what is actually installed -- a machine where ``ndi_install.py`` has not - run sees a smaller bundle than CI does. + Files the calculator packages supply (each one's ``ndi_common``, + found through CalcDoc) are scanned too. """ - root = Path(str(ndi_common_PathConstants.COMMON_FOLDER)) unparseable = [] still_bad = set() - for path in sorted(root.rglob("*.json")): + found = [ + (base, path) for base in _ndi_common_roots() for path in sorted(base.rglob("*.json")) + ] + for root, path in found: # KNOWN_BAD_UPSTREAM_JSON is stored in POSIX form, so the # discovered relative path is normalised to match on Windows too. relative = PurePosixPath(*path.relative_to(root).parts).as_posix() @@ -379,18 +432,25 @@ def test_definitions_validate_or_are_listed(self): ) def test_listed_entries_still_exist(self): - """A renamed or deleted definition must not linger in the list. - - Only NDI's own tree is checked. The dependency-supplied list is not: - those files are absent until ``ndi_install.py`` copies them in, and a - bare checkout must not fail for that. - """ - present = {t for t, _ in _bundled_document_types()} - stale = sorted(set(KNOWN_UNVALIDATABLE_DEFINITIONS) - present) + """A renamed or deleted definition must not linger in either list.""" + own = { + t for t, _ in _document_types_under(Path(str(ndi_common_PathConstants.DOCUMENT_PATH))) + } + stale = sorted(set(KNOWN_UNVALIDATABLE_DEFINITIONS) - own) assert not stale, ( "KNOWN_UNVALIDATABLE_DEFINITIONS names definitions that no longer " "exist: " + ", ".join(stale) ) + if not ndi_common_PathConstants.CalcDoc: + return + calc = { + t for root in ndi_common_PathConstants.CalcDoc for t, _ in _document_types_under(root) + } + stale = sorted(set(KNOWN_UNVALIDATABLE_FROM_DEPENDENCIES) - calc) + assert not stale, ( + "KNOWN_UNVALIDATABLE_FROM_DEPENDENCIES names definitions no " + "installed calculator package has: " + ", ".join(stale) + ) # --------------------------------------------------------------------------- diff --git a/tests/test_document_path_variables.py b/tests/test_document_path_variables.py index f2d57c55..54d7036c 100644 --- a/tests/test_document_path_variables.py +++ b/tests/test_document_path_variables.py @@ -2,8 +2,9 @@ MATLAB registers each one as a DID global (ndi.common.PathConstants), so ``$NDIDOCUMENTPATH`` and ``$NDICALCDOCUMENTPATH`` both resolve there. In -NDI-python they resolve to the same folder, because ndi_install.py copies -each dependency's ``ndi_common`` tree into NDI-python's own. +NDI-python the second names the ``CalcDoc`` folders -- one per installed +calculator package (ndicalc-vis-matlab), found through the ``ndi.calc`` +entry point -- and definition lookup searches NDI's own tree and all of them. Resolution used to strip only ``$NDIDOCUMENTPATH``, leaving ``$NDICALCDOCUMENTPATH/...`` as a literal path that could not be found -- @@ -19,10 +20,9 @@ from ndi.common import ndi_common_PathConstants from ndi.document import _definition_to_doc_type, ndi_document -CALC_DOC = ndi_common_PathConstants.COMMON_FOLDER / "database_documents" / "calc" needs_calc = pytest.mark.skipif( - not CALC_DOC.is_dir(), - reason="NDIcalc-vis-matlab documents are installed by ndi_install.py", + not any((p / "calc").is_dir() for p in ndi_common_PathConstants.CalcDoc), + reason="NDIcalc-vis-matlab documents come from the ndicalc-vis-matlab package", ) diff --git a/tests/test_issues_93_94.py b/tests/test_issues_93_94.py index 22f0d298..4ef8faff 100644 --- a/tests/test_issues_93_94.py +++ b/tests/test_issues_93_94.py @@ -41,9 +41,9 @@ def _schema_dir() -> Path: return Path(ndi.ndi_common.__path__[0]) / "schema_documents" -#: Upstream calc schemas that ``ndi_install.py`` copies in but that are -#: broken at the upstream tag AND at head, so we cannot fix them from this -#: repository. Kept in sync with ``tests/test_did_integration.py``'s +#: Calc schemas the ndicalc-vis-matlab package supplies (found through +#: ``ndi_common_PathConstants.CalcDocSchema``) that are broken at the upstream +#: tag AND at head, so we cannot fix them from this repository. Kept in sync with ``tests/test_did_integration.py``'s #: ``KNOWN_BAD_UPSTREAM_JSON`` and named as ``schema_documents``-relative #: POSIX paths — see the quarantine comment there. _KNOWN_BAD_UPSTREAM_SCHEMAS = { @@ -59,10 +59,12 @@ def test_all_bundled_schemas_parse(): starts parsing, the second assertion below fails and this list must be trimmed so the quarantine cannot rot. """ - root = _schema_dir() + from ndi.common import ndi_common_PathConstants + + roots = [_schema_dir(), *ndi_common_PathConstants.CalcDocSchema] failures: list[tuple[Path, str]] = [] unexpected_parses: list[str] = [] - for path in sorted(root.rglob("*.json")): + for root, path in [(r, p) for r in roots for p in sorted(r.rglob("*.json"))]: rel = path.relative_to(root).as_posix() try: with open(path, encoding="utf-8") as f: diff --git a/tests/test_matlab_bridge_properties.py b/tests/test_matlab_bridge_properties.py index 44061d56..f4d680bf 100644 --- a/tests/test_matlab_bridge_properties.py +++ b/tests/test_matlab_bridge_properties.py @@ -307,7 +307,11 @@ def public_names( names: set[str] = set() for node in index[class_name]: names |= _declared_names(node) - for base in node.bases: + # A metaclass's properties are reachable on the class itself, which + # is how ndi_common_PathConstants mirrors MATLAB's Constant + # properties (CalcDoc, CalcDocSchema), so follow it like a base. + metaclasses = [k.value for k in node.keywords if k.arg == "metaclass"] + for base in [*node.bases, *metaclasses]: name = _base_name(base) if name: names |= public_names(name, index, seen) diff --git a/tests/test_ndi_install_exit_codes.py b/tests/test_ndi_install_exit_codes.py index 458732f8..803dc481 100644 --- a/tests/test_ndi_install_exit_codes.py +++ b/tests/test_ndi_install_exit_codes.py @@ -67,7 +67,6 @@ def _run(module, argv: list[str], **overrides) -> tuple[int, str]: "get_site_packages": lambda: Path("/tmp"), "write_pth_file": lambda *a, **k: Path("/tmp/ndi-test.pth"), "find_ndi_root": lambda: REPO_ROOT, - "install_ndi_common_docs": lambda *a, **k: None, } defaults.update(overrides)