Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,4 @@ site/

# Generated tutorial HTML (keep locally for review, not in repo)
tutorials/*.html
*.sqlite
88 changes: 72 additions & 16 deletions src/ndi/cloud/orchestration.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,29 +113,85 @@ def downloadDataset(
if verbose:
print(f' Files downloaded: {report["downloaded"]}, failed: {report["failed"]}')

# Check how many documents actually made it into the dataset
from ndi.query import ndi_query as _ndi_query
# Collect failures: conversion + exception-tracked + silent (DID-python)
add_failures: list[tuple[str, str]] = list(getattr(dataset, "add_doc_failures", []))

# Cross-check using raw DID-python doc IDs (not isa('base') query,
# which might miss documents whose type info wasn't stored correctly).
db_ids = set(
dataset._session._database._driver._db.get_doc_ids(
dataset._session._database._driver._branch_id
)
)

# Build a map from doc_id -> original JSON for missing-doc output
doc_json_by_id: dict[str, dict] = {}
for dj in doc_jsons:
did = dj.get("base", {}).get("id", "") if isinstance(dj, dict) else ""
if did:
doc_json_by_id[did] = dj

# Find documents that were "added" (no exception) but aren't in the DB
tracked_ids = {f[0] for f in add_failures}
silent_failures: list[str] = []
for doc in documents:
doc_id = (
doc.document_properties.get("base", {}).get("id", "")
if hasattr(doc, "document_properties")
else doc.get("base", {}).get("id", "")
)
if doc_id and doc_id not in db_ids and doc_id not in tracked_ids:
silent_failures.append(doc_id)

db_docs = dataset.database_search(_ndi_query("").isa("base"))
db_count = len(db_docs)
db_lost = len(documents) - db_count
total_lost = conversion_lost + db_lost
total_lost = conversion_lost + len(add_failures) + len(silent_failures)

if verbose:
print("Download complete.")

if total_lost > 0:
parts = []
# Write missing documents to a JSON file for inspection
missing_docs_path = target / "missingDocuments.json"
missing_docs = []
for doc_id in silent_failures:
if doc_id in doc_json_by_id:
missing_docs.append(doc_json_by_id[doc_id])
else:
missing_docs.append({"base": {"id": doc_id}})
for doc_id, reason in add_failures:
entry = dict(doc_json_by_id.get(doc_id, {"base": {"id": doc_id}}))
entry["_add_error"] = reason
missing_docs.append(entry)
if missing_docs:
import json

missing_docs_path.write_text(json.dumps(missing_docs, indent=2, default=str))

lines = [
f"Downloaded {len(doc_jsons)} documents but only "
f"{len(db_ids)} were added to the dataset. "
f"{total_lost} document(s) lost:"
]
if conversion_lost > 0:
parts.append(f"{conversion_lost} failed to convert from JSON to ndi_document")
if db_lost > 0:
parts.append(f"{db_lost} failed to add to the dataset database")
raise RuntimeError(
f"Downloaded {len(doc_jsons)} documents but only {db_count} "
f"were added to the dataset. {total_lost} documents lost: "
+ "; ".join(parts)
+ ". See preceding warnings for details on each failed document."
)
lines.append(f"\n{conversion_lost} failed to convert from JSON" " to ndi_document")
if add_failures:
lines.append(f"\n{len(add_failures)} raised errors during" " database add:")
for doc_id, reason in add_failures[:50]:
lines.append(f"\n - {doc_id}: {reason}")
if len(add_failures) > 50:
lines.append(f"\n ... and {len(add_failures) - 50} more")
if silent_failures:
lines.append(
f"\n{len(silent_failures)} were passed to"
" database.add() without error but are NOT in the"
" database (possible DID-python bug):"
)
for doc_id in silent_failures[:50]:
lines.append(f"\n - {doc_id}")
if len(silent_failures) > 50:
lines.append(f"\n ... and {len(silent_failures) - 50} more")
if missing_docs:
lines.append(f"\nFull JSON of missing documents written to:" f"\n {missing_docs_path}")
raise RuntimeError("".join(lines))

return dataset

Expand Down
40 changes: 20 additions & 20 deletions src/ndi/dataset/_dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,11 +169,10 @@ def add_ingested_session(self, session: Any) -> ndi_dataset:
self._session._database.add(doc)
self._copy_binary_files(session, doc)
except FileExistsError:
pass # Duplicates are expected and safe to skip
pass # Re-ingestion duplicates are expected
except Exception as exc:
doc_id = getattr(doc, "id", "<unknown>")
ingestion_failures.append((str(doc_id), str(exc)))
logger.debug("Skipping document %s during ingestion: %s", doc_id, exc)
doc_id = ndi_dataset_dir._get_doc_id(doc)
ingestion_failures.append((doc_id, str(exc)))
if ingestion_failures:
failure_details = "\n".join(
f" - {doc_id}: {err}" for doc_id, err in ingestion_failures[:20]
Expand Down Expand Up @@ -828,6 +827,10 @@ def __init__(

self._path.mkdir(parents=True, exist_ok=True)

# Track documents that failed to add (list of (doc_id, reason) tuples).
# Callers (e.g. downloadDataset) can inspect this after construction.
self.add_doc_failures: list[tuple[str, str]] = []

if documents is not None and documents:
# Hidden 3rd argument: create from pre-loaded documents.
# Mirrors MATLAB ndi.dataset.dir(reference, path_name, docs).
Expand All @@ -839,26 +842,12 @@ def __init__(
session_id=dataset_session_id,
)
# Bulk-add all documents to the database
failures: list[tuple[str, str]] = []
for doc in documents:
try:
self._session._database.add(doc)
except FileExistsError:
pass # Duplicates are expected and safe to skip
except Exception as exc:
doc_id = (
getattr(doc, "id", None) or doc.get("base", {}).get("id", "<unknown>")
if isinstance(doc, dict)
else "<unknown>"
)
failures.append((str(doc_id), str(exc)))
if failures:
failure_details = "\n".join(f" - {doc_id}: {err}" for doc_id, err in failures[:20])
extra = f"\n ... and {len(failures) - 20} more" if len(failures) > 20 else ""
raise RuntimeError(
f"Failed to add {len(failures)} of {len(documents)} "
f"documents to dataset database:\n{failure_details}{extra}"
)
doc_id = self._get_doc_id(doc)
self.add_doc_failures.append((doc_id, str(exc)))
# Re-create session without forced ID (reads from database)
self._session = ndi_session_dir(ref or "temp", self._path)
elif path_or_ref is None and not ref:
Expand Down Expand Up @@ -1006,6 +995,17 @@ def dataset_erase(ndi_dataset_dir_obj: ndi_dataset_dir, areyousure: str = "no")
"user did not indicate they are sure."
)

@staticmethod
def _get_doc_id(doc: Any) -> str:
"""Best-effort extraction of a document ID for error reporting."""
doc_id = getattr(doc, "id", None)
if doc_id:
return str(doc_id)
props = getattr(doc, "document_properties", doc)
if isinstance(props, dict):
return props.get("base", {}).get("id", "<unknown>")
return "<unknown>"

@staticmethod
def _dataset_session_id_from_docs(documents: list[ndi_document]) -> str:
"""Extract the dataset session ID from a list of documents.
Expand Down
19 changes: 13 additions & 6 deletions src/ndi/document.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,15 +95,16 @@ def __init__(self, document_type: Union[str, dict, "ndi_document"] = "base", **k
self._set_nested_property(key, value)

def _normalize_depends_on(self):
"""Ensure depends_on and files.file_info are always lists.
"""Ensure depends_on, files.file_info, and superclasses are always lists.

MATLAB's jsonencode converts single-element cell arrays to scalars,
so documents downloaded from the cloud (or created by MATLAB) may
have ``depends_on`` or ``files.file_info`` as a bare dict instead
of a list. Normalizing here guarantees that downstream code
(including DID-python's ``doc_to_sql`` / ``_serialize_depends_on``
and all NDI code that iterates over ``file_info``) always sees
a list.
have ``depends_on``, ``files.file_info``, or
``document_class.superclasses`` as a bare dict instead of a list.
Normalizing here guarantees that downstream code (including
DID-python's ``doc_to_sql`` / ``_serialize_depends_on`` /
``_get_superclass_str`` and all NDI code that iterates over these
fields) always sees a list.
"""
dep = self._document_properties.get("depends_on")
if isinstance(dep, dict):
Expand All @@ -115,6 +116,12 @@ def _normalize_depends_on(self):
if isinstance(fi, dict):
files["file_info"] = [fi]

dc = self._document_properties.get("document_class")
if isinstance(dc, dict):
sc = dc.get("superclasses")
if isinstance(sc, dict):
dc["superclasses"] = [sc]

@property
def document_properties(self) -> dict:
"""The document's properties as a nested dictionary."""
Expand Down
Loading