From 57336c51652f2f249ddb96205af1d6cd68d3b417 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Mar 2026 23:37:02 +0000 Subject: [PATCH 1/6] Fix session_list() to return list of dicts instead of tuple The Dataset.session_list() method was returning a tuple of (session_references, session_list) but all callers expected a flat list of dicts. This caused 20 test failures with TypeError and assertion errors about len() returning 2 (tuple size) instead of the session count. https://claude.ai/code/session_01ManyQhMJJszw7Euj7V31Z1 --- src/ndi/database_fun.py | 5 +++-- src/ndi/dataset/_dataset.py | 18 +++++++----------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/ndi/database_fun.py b/src/ndi/database_fun.py index efdb1bc..89d827a 100644 --- a/src/ndi/database_fun.py +++ b/src/ndi/database_fun.py @@ -390,9 +390,10 @@ def copy_session_to_dataset( # Check for already-copied sessions try: - refs, session_ids = ndi_dataset_obj.session_list() + sessions = ndi_dataset_obj.session_list() session_id = ndi_session_obj.id() - if session_id in session_ids: + existing_ids = [s["session_id"] for s in sessions] + if session_id in existing_ids: return ( False, f"Session with ID {session_id} is already part of " f"the dataset.", diff --git a/src/ndi/dataset/_dataset.py b/src/ndi/dataset/_dataset.py index 39de3ce..54cf280 100644 --- a/src/ndi/dataset/_dataset.py +++ b/src/ndi/dataset/_dataset.py @@ -224,28 +224,24 @@ def open_session(self, session_id: str) -> Any | None: return session - def session_list(self) -> tuple[list[str], list[dict[str, Any]]]: + def session_list(self) -> list[dict[str, Any]]: """ List all sessions in this dataset. Returns: - A tuple of (session_references, session_list) where: - - session_references: List of session reference strings - - session_list: List of dicts with keys: - - session_id: Session identifier - - session_reference: Session reference name - - is_linked: True if linked, False if ingested - - document_id: ID of the session_in_a_dataset document + List of dicts with keys: + - session_id: Session identifier + - session_reference: Session reference name + - is_linked: True if linked, False if ingested + - document_id: ID of the session_in_a_dataset document """ q = Query("").isa("session_in_a_dataset") docs = self._session.database_search(q) - session_references = [] session_list = [] for doc in docs: props = doc.document_properties.get("session_in_a_dataset", {}) ref = props.get("session_reference", "") - session_references.append(ref) session_list.append( { "session_id": props.get("session_id", ""), @@ -255,7 +251,7 @@ def session_list(self) -> tuple[list[str], list[dict[str, Any]]]: } ) - return session_references, session_list + return session_list # ========================================================================= # Database Operations (delegated to internal session) From 205c9bbdae99975490f93a126445b5a980af7370 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Mar 2026 23:39:43 +0000 Subject: [PATCH 2/6] Revert "Fix session_list() to return list of dicts instead of tuple" This reverts commit 57336c51652f2f249ddb96205af1d6cd68d3b417. --- src/ndi/database_fun.py | 5 ++--- src/ndi/dataset/_dataset.py | 18 +++++++++++------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/ndi/database_fun.py b/src/ndi/database_fun.py index 89d827a..efdb1bc 100644 --- a/src/ndi/database_fun.py +++ b/src/ndi/database_fun.py @@ -390,10 +390,9 @@ def copy_session_to_dataset( # Check for already-copied sessions try: - sessions = ndi_dataset_obj.session_list() + refs, session_ids = ndi_dataset_obj.session_list() session_id = ndi_session_obj.id() - existing_ids = [s["session_id"] for s in sessions] - if session_id in existing_ids: + if session_id in session_ids: return ( False, f"Session with ID {session_id} is already part of " f"the dataset.", diff --git a/src/ndi/dataset/_dataset.py b/src/ndi/dataset/_dataset.py index 54cf280..39de3ce 100644 --- a/src/ndi/dataset/_dataset.py +++ b/src/ndi/dataset/_dataset.py @@ -224,24 +224,28 @@ def open_session(self, session_id: str) -> Any | None: return session - def session_list(self) -> list[dict[str, Any]]: + def session_list(self) -> tuple[list[str], list[dict[str, Any]]]: """ List all sessions in this dataset. Returns: - List of dicts with keys: - - session_id: Session identifier - - session_reference: Session reference name - - is_linked: True if linked, False if ingested - - document_id: ID of the session_in_a_dataset document + A tuple of (session_references, session_list) where: + - session_references: List of session reference strings + - session_list: List of dicts with keys: + - session_id: Session identifier + - session_reference: Session reference name + - is_linked: True if linked, False if ingested + - document_id: ID of the session_in_a_dataset document """ q = Query("").isa("session_in_a_dataset") docs = self._session.database_search(q) + session_references = [] session_list = [] for doc in docs: props = doc.document_properties.get("session_in_a_dataset", {}) ref = props.get("session_reference", "") + session_references.append(ref) session_list.append( { "session_id": props.get("session_id", ""), @@ -251,7 +255,7 @@ def session_list(self) -> list[dict[str, Any]]: } ) - return session_list + return session_references, session_list # ========================================================================= # Database Operations (delegated to internal session) From 3d1d482a5730f0ab769f7071efc9f0eec0b24fbd Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Mar 2026 23:42:42 +0000 Subject: [PATCH 3/6] Fix tests to match session_list() tuple return type (refs, details) session_list() returns a MATLAB-compatible tuple of (references, details) but tests were treating it as a flat list of dicts. Updated all 20 failing tests to unpack the tuple and use the details list. Also fixed Dataset.__repr__ to use the details list length. https://claude.ai/code/session_01ManyQhMJJszw7Euj7V31Z1 --- src/ndi/dataset/_dataset.py | 4 +-- tests/matlab_tests/test_dataset.py | 55 +++++++++++++++--------------- tests/matlab_tests/test_session.py | 16 ++++----- tests/test_phase8.py | 48 ++++++++++++++------------ 4 files changed, 65 insertions(+), 58 deletions(-) diff --git a/src/ndi/dataset/_dataset.py b/src/ndi/dataset/_dataset.py index 39de3ce..f1bcf7f 100644 --- a/src/ndi/dataset/_dataset.py +++ b/src/ndi/dataset/_dataset.py @@ -465,5 +465,5 @@ def _recreate_linked_session(self, props: dict[str, Any]) -> Any | None: def __repr__(self) -> str: """String representation.""" - sessions = self.session_list() - return f"Dataset('{self._reference}', sessions={len(sessions)})" + _refs, details = self.session_list() + return f"Dataset('{self._reference}', sessions={len(details)})" diff --git a/tests/matlab_tests/test_dataset.py b/tests/matlab_tests/test_dataset.py index a3b1fb0..f1b7c3d 100644 --- a/tests/matlab_tests/test_dataset.py +++ b/tests/matlab_tests/test_dataset.py @@ -123,8 +123,8 @@ def test_setup(self, build_dataset): assert isinstance(session, DirSession) # Session should be in dataset's session list - sessions = dataset.session_list() - session_ids = [s["session_id"] for s in sessions] + refs, details = dataset.session_list() + session_ids = [s["session_id"] for s in details] assert session.id() in session_ids, "Session ID should be in dataset session list" # Should find exactly 5 demoNDI documents @@ -172,12 +172,12 @@ def test_session_list_outputs(self, build_dataset): """ dataset, session = build_dataset - sessions = dataset.session_list() + refs, details = dataset.session_list() # Should have exactly 1 session - assert len(sessions) == 1 + assert len(details) == 1 - entry = sessions[0] + entry = details[0] # 1. Verify session_reference assert ( @@ -239,8 +239,8 @@ def test_delete_success(self, build_dataset): session_id = session.id() # Verify session exists initially - sessions = dataset.session_list() - ids = [s["session_id"] for s in sessions] + refs, details = dataset.session_list() + ids = [s["session_id"] for s in details] assert session_id in ids, "Session ID should be in dataset" # Verify documents exist @@ -252,8 +252,8 @@ def test_delete_success(self, build_dataset): dataset.delete_ingested_session(session_id, are_you_sure=True) # Verify session is removed from list - sessions_after = dataset.session_list() - ids_after = [s["session_id"] for s in sessions_after] + refs_after, details_after = dataset.session_list() + ids_after = [s["session_id"] for s in details_after] assert session_id not in ids_after, "Session ID should NOT be in dataset after deletion" # Verify documents are removed @@ -273,8 +273,8 @@ def test_delete_not_confirmed(self, build_dataset): dataset.delete_ingested_session(session_id, are_you_sure=False) # Verify session still exists - sessions = dataset.session_list() - ids = [s["session_id"] for s in sessions] + refs, details = dataset.session_list() + ids = [s["session_id"] for s in details] assert session_id in ids, "Session ID should still be in dataset after failed delete" def test_delete_linked_session_error(self, build_dataset, tmp_path): @@ -293,8 +293,8 @@ def test_delete_linked_session_error(self, build_dataset, tmp_path): dataset.add_linked_session(linked_session) # Verify it was added - sessions = dataset.session_list() - ids = [s["session_id"] for s in sessions] + refs, details = dataset.session_list() + ids = [s["session_id"] for s in details] assert linked_session.id() in ids # Attempt to delete a linked session — should fail @@ -347,17 +347,17 @@ def test_unlink_linked_session(self, tmp_path): dataset.add_linked_session(session) # Verify session is linked - sessions = dataset.session_list() - assert len(sessions) == 1 - assert sessions[0]["session_id"] == session.id() - assert sessions[0]["is_linked"] is True + refs, details = dataset.session_list() + assert len(details) == 1 + assert details[0]["session_id"] == session.id() + assert details[0]["is_linked"] is True # Unlink dataset.unlink_session(session.id()) # Verify session is gone from dataset - sessions_after = dataset.session_list() - assert len(sessions_after) == 0, "Session list should be empty after unlink" + refs_after, details_after = dataset.session_list() + assert len(details_after) == 0, "Session list should be empty after unlink" # Session files should still exist assert ( @@ -397,8 +397,8 @@ def test_unlink_with_remove_documents(self, tmp_path): dataset.unlink_session(session_id, remove_documents=True) # Verify session removed from list - sessions_after = dataset.session_list() - assert len(sessions_after) == 0, "Session list should be empty" + refs_after, details_after = dataset.session_list() + assert len(details_after) == 0, "Session list should be empty" # Verify documents removed from dataset docs_after = dataset.database_search(q) @@ -430,8 +430,8 @@ def test_unlink_ingested_session(self, tmp_path): dataset.unlink_session(session_id) # Verify session removed from list - sessions_after = dataset.session_list() - assert len(sessions_after) == 0 + refs_after, details_after = dataset.session_list() + assert len(details_after) == 0 def test_unlink_nonexistent_session(self, tmp_path): """Unlinking a nonexistent session does nothing. @@ -447,7 +447,8 @@ def test_unlink_nonexistent_session(self, tmp_path): dataset.unlink_session("nonexistent_id") # Verify still empty - assert len(dataset.session_list()) == 0 + refs, details = dataset.session_list() + assert len(details) == 0 # =========================================================================== @@ -494,10 +495,10 @@ def test_open_existing_dataset(self, tmp_path): assert reopened.id() == original_ds_id, "Reopened dataset should have same ID" # 4. Verify session list - sessions = reopened.session_list() - assert len(sessions) >= 1, "Reopened dataset should have at least 1 session" + refs, details = reopened.session_list() + assert len(details) >= 1, "Reopened dataset should have at least 1 session" - session_ids = [s["session_id"] for s in sessions] + session_ids = [s["session_id"] for s in details] assert ( original_session_id in session_ids ), "Original session should still be in the reopened dataset" diff --git a/tests/matlab_tests/test_session.py b/tests/matlab_tests/test_session.py index 63ffa89..ed39c62 100644 --- a/tests/matlab_tests/test_session.py +++ b/tests/matlab_tests/test_session.py @@ -105,8 +105,8 @@ def test_standalone_session_not_in_dataset(self, tmp_path): ds_dir.mkdir() dataset = Dataset(ds_dir, "ds_empty") - sessions = dataset.session_list() - session_ids = [s["session_id"] for s in sessions] + refs, details = dataset.session_list() + session_ids = [s["session_id"] for s in details] assert session.id() not in session_ids, "Standalone session should not be in dataset" def test_ingested_session_in_dataset(self, tmp_path): @@ -133,8 +133,8 @@ def test_ingested_session_in_dataset(self, tmp_path): dataset = Dataset(ds_dir, "ds") dataset.add_ingested_session(session) - sessions = dataset.session_list() - session_ids = [s["session_id"] for s in sessions] + refs, details = dataset.session_list() + session_ids = [s["session_id"] for s in details] assert session.id() in session_ids, "Ingested session should appear in dataset session list" # Also verify the document was ingested @@ -154,10 +154,10 @@ def test_linked_session_in_dataset(self, tmp_path): dataset = Dataset(ds_dir, "ds_link") dataset.add_linked_session(session) - sessions = dataset.session_list() - assert len(sessions) == 1 - assert sessions[0]["session_id"] == session.id() - assert sessions[0]["is_linked"] is True + refs, details = dataset.session_list() + assert len(details) == 1 + assert details[0]["session_id"] == session.id() + assert details[0]["is_linked"] is True # =========================================================================== diff --git a/tests/test_phase8.py b/tests/test_phase8.py index 23cbb33..509c3a3 100644 --- a/tests/test_phase8.py +++ b/tests/test_phase8.py @@ -409,10 +409,10 @@ def test_add_linked_session(self, temp_dir, session): ds = Dataset(temp_dir / "dataset1", "Test") ds.add_linked_session(session) - sessions = ds.session_list() - assert len(sessions) == 1 - assert sessions[0]["session_id"] == session.id() - assert sessions[0]["is_linked"] is True + refs, details = ds.session_list() + assert len(details) == 1 + assert details[0]["session_id"] == session.id() + assert details[0]["is_linked"] is True def test_add_multiple_sessions(self, temp_dir, session, session2): """Test adding multiple sessions.""" @@ -420,8 +420,8 @@ def test_add_multiple_sessions(self, temp_dir, session, session2): ds.add_linked_session(session) ds.add_linked_session(session2) - sessions = ds.session_list() - assert len(sessions) == 2 + refs, details = ds.session_list() + assert len(details) == 2 def test_add_duplicate_session(self, temp_dir, session): """Test that adding same session twice doesn't duplicate.""" @@ -429,8 +429,8 @@ def test_add_duplicate_session(self, temp_dir, session): ds.add_linked_session(session) ds.add_linked_session(session) - sessions = ds.session_list() - assert len(sessions) == 1 + refs, details = ds.session_list() + assert len(details) == 1 def test_add_ingested_session(self, temp_dir, session): """Test ingesting a session.""" @@ -442,18 +442,20 @@ def test_add_ingested_session(self, temp_dir, session): ds = Dataset(temp_dir / "dataset1", "Test") ds.add_ingested_session(session) - sessions = ds.session_list() - assert len(sessions) == 1 - assert sessions[0]["is_linked"] is False + refs, details = ds.session_list() + assert len(details) == 1 + assert details[0]["is_linked"] is False def test_unlink_session(self, temp_dir, session): """Test unlinking a session.""" ds = Dataset(temp_dir / "dataset1", "Test") ds.add_linked_session(session) - assert len(ds.session_list()) == 1 + refs, details = ds.session_list() + assert len(details) == 1 ds.unlink_session(session.id()) - assert len(ds.session_list()) == 0 + refs, details = ds.session_list() + assert len(details) == 0 def test_unlink_nonexistent(self, temp_dir): """Test unlinking a session that doesn't exist (no error).""" @@ -463,16 +465,18 @@ def test_unlink_nonexistent(self, temp_dir): def test_session_list_empty(self, temp_dir): """Test session list on empty dataset.""" ds = Dataset(temp_dir / "dataset1", "Test") - assert ds.session_list() == [] + refs, details = ds.session_list() + assert refs == [] + assert details == [] def test_session_list_details(self, temp_dir, session): """Test session list returns correct details.""" ds = Dataset(temp_dir / "dataset1", "Test") ds.add_linked_session(session) - sessions = ds.session_list() - assert len(sessions) == 1 - entry = sessions[0] + refs, details = ds.session_list() + assert len(details) == 1 + entry = details[0] assert "session_id" in entry assert "session_reference" in entry assert "is_linked" in entry @@ -518,10 +522,12 @@ def test_delete_ingested_session(self, temp_dir, session): """Test deleting an ingested session.""" ds = Dataset(temp_dir / "dataset1", "Test") ds.add_ingested_session(session) - assert len(ds.session_list()) == 1 + refs, details = ds.session_list() + assert len(details) == 1 ds.delete_ingested_session(session.id(), are_you_sure=True) - assert len(ds.session_list()) == 0 + refs, details = ds.session_list() + assert len(details) == 0 def test_delete_linked_session_raises(self, temp_dir, session): """Test that deleting a linked session raises error.""" @@ -598,8 +604,8 @@ def test_dataset_with_sessions_and_subjects(self, temp_dir, session, session2): ds.add_linked_session(session2) # Verify - sessions = ds.session_list() - assert len(sessions) == 2 + refs, details = ds.session_list() + assert len(details) == 2 except FileNotFoundError: pytest.skip("Schema not available") From 51789cf550a811f45fe913d342ae4d61ab3b4124 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Mar 2026 23:45:49 +0000 Subject: [PATCH 4/6] Change session_list() to return (refs, session_ids) tuple of two lists Match MATLAB convention where session_list() returns two cell arrays: session references and session IDs. Updated tests to unpack the tuple and use the appropriate list. Also fixed Dataset.__repr__ and the database_fun.py caller. https://claude.ai/code/session_01ManyQhMJJszw7Euj7V31Z1 --- src/ndi/dataset/_dataset.py | 32 ++++-------- tests/matlab_tests/test_dataset.py | 78 ++++++++++++------------------ tests/matlab_tests/test_session.py | 13 ++--- tests/test_phase8.py | 58 +++++++++++----------- 4 files changed, 72 insertions(+), 109 deletions(-) diff --git a/src/ndi/dataset/_dataset.py b/src/ndi/dataset/_dataset.py index f1bcf7f..2b3600f 100644 --- a/src/ndi/dataset/_dataset.py +++ b/src/ndi/dataset/_dataset.py @@ -224,38 +224,26 @@ def open_session(self, session_id: str) -> Any | None: return session - def session_list(self) -> tuple[list[str], list[dict[str, Any]]]: + def session_list(self) -> tuple[list[str], list[str]]: """ List all sessions in this dataset. Returns: - A tuple of (session_references, session_list) where: + A tuple of (session_references, session_ids) where: - session_references: List of session reference strings - - session_list: List of dicts with keys: - - session_id: Session identifier - - session_reference: Session reference name - - is_linked: True if linked, False if ingested - - document_id: ID of the session_in_a_dataset document + - session_ids: List of session ID strings """ q = Query("").isa("session_in_a_dataset") docs = self._session.database_search(q) session_references = [] - session_list = [] + session_ids = [] for doc in docs: props = doc.document_properties.get("session_in_a_dataset", {}) - ref = props.get("session_reference", "") - session_references.append(ref) - session_list.append( - { - "session_id": props.get("session_id", ""), - "session_reference": ref, - "is_linked": bool(props.get("is_linked", False)), - "document_id": doc.id, - } - ) - - return session_references, session_list + session_references.append(props.get("session_reference", "")) + session_ids.append(props.get("session_id", "")) + + return session_references, session_ids # ========================================================================= # Database Operations (delegated to internal session) @@ -465,5 +453,5 @@ def _recreate_linked_session(self, props: dict[str, Any]) -> Any | None: def __repr__(self) -> str: """String representation.""" - _refs, details = self.session_list() - return f"Dataset('{self._reference}', sessions={len(details)})" + refs, _ids = self.session_list() + return f"Dataset('{self._reference}', sessions={len(refs)})" diff --git a/tests/matlab_tests/test_dataset.py b/tests/matlab_tests/test_dataset.py index f1b7c3d..bba7aae 100644 --- a/tests/matlab_tests/test_dataset.py +++ b/tests/matlab_tests/test_dataset.py @@ -123,8 +123,7 @@ def test_setup(self, build_dataset): assert isinstance(session, DirSession) # Session should be in dataset's session list - refs, details = dataset.session_list() - session_ids = [s["session_id"] for s in details] + refs, session_ids = dataset.session_list() assert session.id() in session_ids, "Session ID should be in dataset session list" # Should find exactly 5 demoNDI documents @@ -157,8 +156,7 @@ def test_setup(self, build_dataset): # Port of: ndi.unittest.dataset.testSessionList # # NOTE: MATLAB session_list() returns (refs, ids, sess_docs, dset_doc). -# Python session_list() returns List[Dict] with keys: -# session_id, session_reference, is_linked, document_id +# Python session_list() returns (refs, session_ids) — two lists of strings. # =========================================================================== @@ -172,33 +170,24 @@ def test_session_list_outputs(self, build_dataset): """ dataset, session = build_dataset - refs, details = dataset.session_list() + refs, session_ids = dataset.session_list() # Should have exactly 1 session - assert len(details) == 1 - - entry = details[0] + assert len(session_ids) == 1 + assert len(refs) == 1 # 1. Verify session_reference - assert ( - entry["session_reference"] == "exp_demo" - ), "Session reference should match expected value" + assert refs[0] == "exp_demo", "Session reference should match expected value" # 2. Verify session_id assert ( - entry["session_id"] == session.id() + session_ids[0] == session.id() ), "Session ID should match the ingested session ID" - # 3. Verify document_id is present - assert entry["document_id"], "document_id should be non-empty" - - # 4. Verify is_linked is False (this was ingested) - assert entry["is_linked"] is False - - # 5. Verify the session_in_a_dataset document exists and is correct - q = Query("base.id") == entry["document_id"] + # 3. Verify the session_in_a_dataset document exists and is correct + q = Query("").isa("session_in_a_dataset") found = dataset.database_search(q) - assert len(found) == 1, "Should find exactly one document for the session_in_a_dataset ID" + assert len(found) == 1, "Should find exactly one session_in_a_dataset document" doc = found[0] props = doc.document_properties.get("session_in_a_dataset", {}) @@ -239,9 +228,8 @@ def test_delete_success(self, build_dataset): session_id = session.id() # Verify session exists initially - refs, details = dataset.session_list() - ids = [s["session_id"] for s in details] - assert session_id in ids, "Session ID should be in dataset" + refs, session_ids = dataset.session_list() + assert session_id in session_ids, "Session ID should be in dataset" # Verify documents exist q = Query("base.session_id") == session_id @@ -252,8 +240,7 @@ def test_delete_success(self, build_dataset): dataset.delete_ingested_session(session_id, are_you_sure=True) # Verify session is removed from list - refs_after, details_after = dataset.session_list() - ids_after = [s["session_id"] for s in details_after] + refs_after, ids_after = dataset.session_list() assert session_id not in ids_after, "Session ID should NOT be in dataset after deletion" # Verify documents are removed @@ -273,9 +260,8 @@ def test_delete_not_confirmed(self, build_dataset): dataset.delete_ingested_session(session_id, are_you_sure=False) # Verify session still exists - refs, details = dataset.session_list() - ids = [s["session_id"] for s in details] - assert session_id in ids, "Session ID should still be in dataset after failed delete" + refs, session_ids = dataset.session_list() + assert session_id in session_ids, "Session ID should still be in dataset after failed delete" def test_delete_linked_session_error(self, build_dataset, tmp_path): """Deleting a linked session raises ValueError. @@ -293,9 +279,8 @@ def test_delete_linked_session_error(self, build_dataset, tmp_path): dataset.add_linked_session(linked_session) # Verify it was added - refs, details = dataset.session_list() - ids = [s["session_id"] for s in details] - assert linked_session.id() in ids + refs, session_ids = dataset.session_list() + assert linked_session.id() in session_ids # Attempt to delete a linked session — should fail with pytest.raises(ValueError, match="linked"): @@ -347,17 +332,16 @@ def test_unlink_linked_session(self, tmp_path): dataset.add_linked_session(session) # Verify session is linked - refs, details = dataset.session_list() - assert len(details) == 1 - assert details[0]["session_id"] == session.id() - assert details[0]["is_linked"] is True + refs, session_ids = dataset.session_list() + assert len(session_ids) == 1 + assert session_ids[0] == session.id() # Unlink dataset.unlink_session(session.id()) # Verify session is gone from dataset - refs_after, details_after = dataset.session_list() - assert len(details_after) == 0, "Session list should be empty after unlink" + refs_after, ids_after = dataset.session_list() + assert len(ids_after) == 0, "Session list should be empty after unlink" # Session files should still exist assert ( @@ -397,8 +381,8 @@ def test_unlink_with_remove_documents(self, tmp_path): dataset.unlink_session(session_id, remove_documents=True) # Verify session removed from list - refs_after, details_after = dataset.session_list() - assert len(details_after) == 0, "Session list should be empty" + refs_after, ids_after = dataset.session_list() + assert len(ids_after) == 0, "Session list should be empty" # Verify documents removed from dataset docs_after = dataset.database_search(q) @@ -430,8 +414,8 @@ def test_unlink_ingested_session(self, tmp_path): dataset.unlink_session(session_id) # Verify session removed from list - refs_after, details_after = dataset.session_list() - assert len(details_after) == 0 + refs_after, ids_after = dataset.session_list() + assert len(ids_after) == 0 def test_unlink_nonexistent_session(self, tmp_path): """Unlinking a nonexistent session does nothing. @@ -447,8 +431,8 @@ def test_unlink_nonexistent_session(self, tmp_path): dataset.unlink_session("nonexistent_id") # Verify still empty - refs, details = dataset.session_list() - assert len(details) == 0 + refs, session_ids = dataset.session_list() + assert len(session_ids) == 0 # =========================================================================== @@ -495,10 +479,8 @@ def test_open_existing_dataset(self, tmp_path): assert reopened.id() == original_ds_id, "Reopened dataset should have same ID" # 4. Verify session list - refs, details = reopened.session_list() - assert len(details) >= 1, "Reopened dataset should have at least 1 session" - - session_ids = [s["session_id"] for s in details] + refs, session_ids = reopened.session_list() + assert len(session_ids) >= 1, "Reopened dataset should have at least 1 session" assert ( original_session_id in session_ids ), "Original session should still be in the reopened dataset" diff --git a/tests/matlab_tests/test_session.py b/tests/matlab_tests/test_session.py index ed39c62..8b9e054 100644 --- a/tests/matlab_tests/test_session.py +++ b/tests/matlab_tests/test_session.py @@ -105,8 +105,7 @@ def test_standalone_session_not_in_dataset(self, tmp_path): ds_dir.mkdir() dataset = Dataset(ds_dir, "ds_empty") - refs, details = dataset.session_list() - session_ids = [s["session_id"] for s in details] + refs, session_ids = dataset.session_list() assert session.id() not in session_ids, "Standalone session should not be in dataset" def test_ingested_session_in_dataset(self, tmp_path): @@ -133,8 +132,7 @@ def test_ingested_session_in_dataset(self, tmp_path): dataset = Dataset(ds_dir, "ds") dataset.add_ingested_session(session) - refs, details = dataset.session_list() - session_ids = [s["session_id"] for s in details] + refs, session_ids = dataset.session_list() assert session.id() in session_ids, "Ingested session should appear in dataset session list" # Also verify the document was ingested @@ -154,10 +152,9 @@ def test_linked_session_in_dataset(self, tmp_path): dataset = Dataset(ds_dir, "ds_link") dataset.add_linked_session(session) - refs, details = dataset.session_list() - assert len(details) == 1 - assert details[0]["session_id"] == session.id() - assert details[0]["is_linked"] is True + refs, session_ids = dataset.session_list() + assert len(session_ids) == 1 + assert session_ids[0] == session.id() # =========================================================================== diff --git a/tests/test_phase8.py b/tests/test_phase8.py index 509c3a3..fb3fd3f 100644 --- a/tests/test_phase8.py +++ b/tests/test_phase8.py @@ -409,10 +409,9 @@ def test_add_linked_session(self, temp_dir, session): ds = Dataset(temp_dir / "dataset1", "Test") ds.add_linked_session(session) - refs, details = ds.session_list() - assert len(details) == 1 - assert details[0]["session_id"] == session.id() - assert details[0]["is_linked"] is True + refs, session_ids = ds.session_list() + assert len(session_ids) == 1 + assert session_ids[0] == session.id() def test_add_multiple_sessions(self, temp_dir, session, session2): """Test adding multiple sessions.""" @@ -420,8 +419,8 @@ def test_add_multiple_sessions(self, temp_dir, session, session2): ds.add_linked_session(session) ds.add_linked_session(session2) - refs, details = ds.session_list() - assert len(details) == 2 + refs, session_ids = ds.session_list() + assert len(session_ids) == 2 def test_add_duplicate_session(self, temp_dir, session): """Test that adding same session twice doesn't duplicate.""" @@ -429,8 +428,8 @@ def test_add_duplicate_session(self, temp_dir, session): ds.add_linked_session(session) ds.add_linked_session(session) - refs, details = ds.session_list() - assert len(details) == 1 + refs, session_ids = ds.session_list() + assert len(session_ids) == 1 def test_add_ingested_session(self, temp_dir, session): """Test ingesting a session.""" @@ -442,20 +441,19 @@ def test_add_ingested_session(self, temp_dir, session): ds = Dataset(temp_dir / "dataset1", "Test") ds.add_ingested_session(session) - refs, details = ds.session_list() - assert len(details) == 1 - assert details[0]["is_linked"] is False + refs, session_ids = ds.session_list() + assert len(session_ids) == 1 def test_unlink_session(self, temp_dir, session): """Test unlinking a session.""" ds = Dataset(temp_dir / "dataset1", "Test") ds.add_linked_session(session) - refs, details = ds.session_list() - assert len(details) == 1 + refs, session_ids = ds.session_list() + assert len(session_ids) == 1 ds.unlink_session(session.id()) - refs, details = ds.session_list() - assert len(details) == 0 + refs, session_ids = ds.session_list() + assert len(session_ids) == 0 def test_unlink_nonexistent(self, temp_dir): """Test unlinking a session that doesn't exist (no error).""" @@ -465,22 +463,20 @@ def test_unlink_nonexistent(self, temp_dir): def test_session_list_empty(self, temp_dir): """Test session list on empty dataset.""" ds = Dataset(temp_dir / "dataset1", "Test") - refs, details = ds.session_list() + refs, session_ids = ds.session_list() assert refs == [] - assert details == [] + assert session_ids == [] def test_session_list_details(self, temp_dir, session): - """Test session list returns correct details.""" + """Test session list returns refs and ids.""" ds = Dataset(temp_dir / "dataset1", "Test") ds.add_linked_session(session) - refs, details = ds.session_list() - assert len(details) == 1 - entry = details[0] - assert "session_id" in entry - assert "session_reference" in entry - assert "is_linked" in entry - assert "document_id" in entry + refs, session_ids = ds.session_list() + assert len(session_ids) == 1 + assert len(refs) == 1 + assert isinstance(refs[0], str) + assert isinstance(session_ids[0], str) class TestDatasetDatabase: @@ -522,12 +518,12 @@ def test_delete_ingested_session(self, temp_dir, session): """Test deleting an ingested session.""" ds = Dataset(temp_dir / "dataset1", "Test") ds.add_ingested_session(session) - refs, details = ds.session_list() - assert len(details) == 1 + refs, session_ids = ds.session_list() + assert len(session_ids) == 1 ds.delete_ingested_session(session.id(), are_you_sure=True) - refs, details = ds.session_list() - assert len(details) == 0 + refs, session_ids = ds.session_list() + assert len(session_ids) == 0 def test_delete_linked_session_raises(self, temp_dir, session): """Test that deleting a linked session raises error.""" @@ -604,8 +600,8 @@ def test_dataset_with_sessions_and_subjects(self, temp_dir, session, session2): ds.add_linked_session(session2) # Verify - refs, details = ds.session_list() - assert len(details) == 2 + refs, session_ids = ds.session_list() + assert len(session_ids) == 2 except FileNotFoundError: pytest.skip("Schema not available") From ad245e7042617770c1b04e141ea37cf3cf832b76 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Mar 2026 23:58:30 +0000 Subject: [PATCH 5/6] Align Dataset API with MATLAB ndi.dataset updates - session_list() now returns 4 values (ref_list, id_list, session_doc_ids, dataset_session_doc_id) matching MATLAB signature - database_search() now searches linked sessions in addition to internal DB - add_linked_session() raises ValueError on duplicate (was silent) - add_ingested_session() checks is_fully_ingested() before ingesting - unlink_session() requires are_you_sure=True and only works on linked sessions (raises ValueError for ingested sessions) - delete_ingested_session() raises ValueError when session not found - Add _open_linked_sessions() helper for lazy session opening - Update all tests to match new 4-value return and stricter error handling https://claude.ai/code/session_01ManyQhMJJszw7Euj7V31Z1 --- src/ndi/database_fun.py | 2 +- src/ndi/dataset/_dataset.py | 148 ++++++++++++++++++++++++----- tests/matlab_tests/test_dataset.py | 128 +++++++++---------------- tests/matlab_tests/test_session.py | 6 +- tests/test_phase1_gaps.py | 4 +- tests/test_phase8.py | 42 ++++---- 6 files changed, 199 insertions(+), 131 deletions(-) diff --git a/src/ndi/database_fun.py b/src/ndi/database_fun.py index efdb1bc..35f8e1d 100644 --- a/src/ndi/database_fun.py +++ b/src/ndi/database_fun.py @@ -390,7 +390,7 @@ def copy_session_to_dataset( # Check for already-copied sessions try: - refs, session_ids = ndi_dataset_obj.session_list() + refs, session_ids, *_ = ndi_dataset_obj.session_list() session_id = ndi_session_obj.id() if session_id in session_ids: return ( diff --git a/src/ndi/dataset/_dataset.py b/src/ndi/dataset/_dataset.py index 2b3600f..e55d5d0 100644 --- a/src/ndi/dataset/_dataset.py +++ b/src/ndi/dataset/_dataset.py @@ -108,16 +108,24 @@ def add_linked_session(self, session: Any) -> Dataset: Returns: self for chaining + + Raises: + ValueError: If the session is already part of this dataset """ - # Check if already linked existing = self._find_session_doc(session.id()) if existing is not None: - return self + raise ValueError( + f"Session with id {session.id()} is already part of " + f"dataset {self.id()}." + ) # Create session_in_a_dataset document doc = self._create_session_doc(session, is_linked=True) self._session.database_add(doc) + # Cache the session object + self._session_cache[session.id()] = session + return self def add_ingested_session(self, session: Any) -> Dataset: @@ -132,11 +140,26 @@ def add_ingested_session(self, session: Any) -> Dataset: Returns: self for chaining + + Raises: + ValueError: If the session is already part of this dataset + ValueError: If the session is not fully ingested """ - # Check if already present existing = self._find_session_doc(session.id()) if existing is not None: - return self + raise ValueError( + f"Session with id {session.id()} is already part of " + f"dataset {self.id()}." + ) + + # Check if session is fully ingested + if hasattr(session, "is_fully_ingested") and not session.is_fully_ingested(): + raise ValueError( + f"Session with id {session.id()} and reference " + f"{session.reference} is not yet fully ingested. " + f"It must be fully ingested before it can be added " + f"in ingested form to a dataset." + ) # Copy all documents from source session into the dataset's database. # We bypass session.database_add() because it enforces session_id == @@ -160,25 +183,41 @@ def add_ingested_session(self, session: Any) -> Dataset: def unlink_session( self, session_id: str, - remove_documents: bool = False, + are_you_sure: bool = False, ) -> Dataset: """ - Remove a session from this dataset. + Unlink a linked session from this dataset. + + The session must be a linked session (not ingested). Use + delete_ingested_session() for ingested sessions. Args: session_id: ID of the session to unlink - remove_documents: If True, also remove ingested documents + are_you_sure: Must be True to proceed Returns: self for chaining + + Raises: + ValueError: If not confirmed, session not found, or session is ingested """ + if not are_you_sure: + raise ValueError("Must set are_you_sure=True to unlink a session.") + doc = self._find_session_doc(session_id) if doc is None: - return self + raise ValueError( + f"Session with ID {session_id} not found in " + f"dataset {self.id()}." + ) - # Optionally remove ingested documents - if remove_documents: - self._remove_session_documents(session_id) + props = doc.document_properties.get("session_in_a_dataset", {}) + if not props.get("is_linked", False): + raise ValueError( + f"Session with ID {session_id} is an INGESTED session, " + f"not a linked session. Cannot unlink. Use " + f"delete_ingested_session() instead." + ) # Remove the session_in_a_dataset document self._session.database_rm(doc) @@ -224,26 +263,47 @@ def open_session(self, session_id: str) -> Any | None: return session - def session_list(self) -> tuple[list[str], list[str]]: + def session_list( + self, + ) -> tuple[list[str], list[str], list[str], str]: """ List all sessions in this dataset. Returns: - A tuple of (session_references, session_ids) where: - - session_references: List of session reference strings - - session_ids: List of session ID strings + A tuple of (ref_list, id_list, session_doc_ids, dataset_session_doc_id): + - ref_list: List of session reference strings + - id_list: List of session ID strings + - session_doc_ids: List of document IDs for the + session_in_a_dataset documents + - dataset_session_doc_id: Document ID of the dataset's + own session document (empty string if not found) """ q = Query("").isa("session_in_a_dataset") docs = self._session.database_search(q) - session_references = [] - session_ids = [] + ref_list: list[str] = [] + id_list: list[str] = [] + session_doc_ids: list[str] = [] for doc in docs: props = doc.document_properties.get("session_in_a_dataset", {}) - session_references.append(props.get("session_reference", "")) - session_ids.append(props.get("session_id", "")) + ref_list.append(props.get("session_reference", "")) + id_list.append(props.get("session_id", "")) + session_doc_ids.append(doc.id) + + # Find the dataset's own session document + dataset_session_doc_id = "" + q_ds = Query("").isa("session") & ( + Query("base.session_id") == self.id() + ) + ds_docs = self._session.database_search(q_ds) + if len(ds_docs) == 1: + dataset_session_doc_id = ds_docs[0].id + elif len(ds_docs) > 1: + raise ValueError( + "More than 1 session document for the dataset session found." + ) - return session_references, session_ids + return ref_list, id_list, session_doc_ids, dataset_session_doc_id # ========================================================================= # Database Operations (delegated to internal session) @@ -264,14 +324,34 @@ def database_rm( return self def database_search(self, query: Query) -> list[Document]: - """Search the dataset database. + """Search the dataset database and all linked sessions. Unlike Session.database_search(), this does NOT filter by session_id because a dataset stores documents from multiple ingested sessions. + Results from linked sessions are also included. """ if self._session._database is None: - return [] - return self._session._database.search(query) + results: list[Document] = [] + else: + results = list(self._session._database.search(query)) + + # Also search linked sessions + self._open_linked_sessions() + q = Query("").isa("session_in_a_dataset") + info_docs = self._session.database_search(q) + for info_doc in info_docs: + props = info_doc.document_properties.get("session_in_a_dataset", {}) + if props.get("is_linked", False): + sid = props.get("session_id", "") + session = self._session_cache.get(sid) + if session is not None: + try: + linked_results = session.database_search(query) + results.extend(linked_results) + except Exception: + pass + + return results def database_openbinarydoc( self, @@ -312,11 +392,16 @@ def delete_ingested_session( doc = self._find_session_doc(session_id) if doc is None: - return self + raise ValueError( + f"Session {session_id} not found in dataset." + ) props = doc.document_properties.get("session_in_a_dataset", {}) if props.get("is_linked", False): - raise ValueError("Cannot delete a linked session. Use unlink_session() instead.") + raise ValueError( + f"Session {session_id} is a linked session, not an " + f"ingested one. Use unlink_session() instead." + ) # Remove all documents from this session self._remove_session_documents(session_id) @@ -420,6 +505,17 @@ def _remove_session_documents(self, session_id: str) -> None: except Exception as exc: logger.warning("Failed to remove document %s: %s", doc.id, exc) + def _open_linked_sessions(self) -> None: + """Ensure all linked sessions are open and cached.""" + q = Query("").isa("session_in_a_dataset") + docs = self._session.database_search(q) + for doc in docs: + props = doc.document_properties.get("session_in_a_dataset", {}) + if props.get("is_linked", False): + sid = props.get("session_id", "") + if sid and sid not in self._session_cache: + self.open_session(sid) + def _recreate_linked_session(self, props: dict[str, Any]) -> Any | None: """Recreate a linked session from stored creator args.""" creator = props.get("session_creator", "") @@ -453,5 +549,5 @@ def _recreate_linked_session(self, props: dict[str, Any]) -> Any | None: def __repr__(self) -> str: """String representation.""" - refs, _ids = self.session_list() + refs, _ids, _doc_ids, _ds_doc_id = self.session_list() return f"Dataset('{self._reference}', sessions={len(refs)})" diff --git a/tests/matlab_tests/test_dataset.py b/tests/matlab_tests/test_dataset.py index bba7aae..297217f 100644 --- a/tests/matlab_tests/test_dataset.py +++ b/tests/matlab_tests/test_dataset.py @@ -123,7 +123,7 @@ def test_setup(self, build_dataset): assert isinstance(session, DirSession) # Session should be in dataset's session list - refs, session_ids = dataset.session_list() + refs, session_ids, *_ = dataset.session_list() assert session.id() in session_ids, "Session ID should be in dataset session list" # Should find exactly 5 demoNDI documents @@ -155,8 +155,8 @@ def test_setup(self, build_dataset): # TestSessionList # Port of: ndi.unittest.dataset.testSessionList # -# NOTE: MATLAB session_list() returns (refs, ids, sess_docs, dset_doc). -# Python session_list() returns (refs, session_ids) — two lists of strings. +# MATLAB session_list() returns (refs, ids, sess_docs, dset_doc). +# Python session_list() now also returns 4 values to match. # =========================================================================== @@ -170,7 +170,7 @@ def test_session_list_outputs(self, build_dataset): """ dataset, session = build_dataset - refs, session_ids = dataset.session_list() + refs, session_ids, *_ = dataset.session_list() # Should have exactly 1 session assert len(session_ids) == 1 @@ -228,7 +228,7 @@ def test_delete_success(self, build_dataset): session_id = session.id() # Verify session exists initially - refs, session_ids = dataset.session_list() + refs, session_ids, *_ = dataset.session_list() assert session_id in session_ids, "Session ID should be in dataset" # Verify documents exist @@ -240,7 +240,7 @@ def test_delete_success(self, build_dataset): dataset.delete_ingested_session(session_id, are_you_sure=True) # Verify session is removed from list - refs_after, ids_after = dataset.session_list() + refs_after, ids_after, *_ = dataset.session_list() assert session_id not in ids_after, "Session ID should NOT be in dataset after deletion" # Verify documents are removed @@ -260,7 +260,7 @@ def test_delete_not_confirmed(self, build_dataset): dataset.delete_ingested_session(session_id, are_you_sure=False) # Verify session still exists - refs, session_ids = dataset.session_list() + refs, session_ids, *_ = dataset.session_list() assert session_id in session_ids, "Session ID should still be in dataset after failed delete" def test_delete_linked_session_error(self, build_dataset, tmp_path): @@ -279,7 +279,7 @@ def test_delete_linked_session_error(self, build_dataset, tmp_path): dataset.add_linked_session(linked_session) # Verify it was added - refs, session_ids = dataset.session_list() + refs, session_ids, *_ = dataset.session_list() assert linked_session.id() in session_ids # Attempt to delete a linked session — should fail @@ -287,27 +287,23 @@ def test_delete_linked_session_error(self, build_dataset, tmp_path): dataset.delete_ingested_session(linked_session.id(), are_you_sure=True) def test_delete_nonexistent_session(self, build_dataset): - """Deleting a nonexistent session. + """Deleting a nonexistent session raises ValueError. - NOTE: Python returns self silently (no error). - MATLAB raises ndi:dataset:deleteIngestedSession:notFound. - We just verify it doesn't crash. + MATLAB equivalent: testDeleteIngestedSession.testDeleteNotFound + (MATLAB error ID: ndi:dataset:deleteIngestedSession:notFound) """ dataset, _ = build_dataset - # Should not raise - dataset.delete_ingested_session("fake_id_xyz", are_you_sure=True) + with pytest.raises(ValueError, match="not found"): + dataset.delete_ingested_session("fake_id_xyz", are_you_sure=True) # =========================================================================== # TestUnlinkSession # Port of: ndi.unittest.dataset.testUnlinkSession # -# NOTE: Python API differences: -# - MATLAB: unlink_session(id, 'areYouSure', true, 'askUserToConfirm', false, -# 'AlsoDeleteSessionAfterUnlinking', true, 'DeleteSessionAskToConfirm', false) -# - Python: unlink_session(id, remove_documents=False) -# - Python doesn't have areYouSure for unlink -# - Python doesn't delete session files — only removes from dataset +# MATLAB: unlink_session(id, 'areYouSure', true, 'askUserToConfirm', false, ...) +# Python: unlink_session(id, are_you_sure=True) +# Both require confirmation and only allow unlinking linked sessions. # =========================================================================== @@ -332,15 +328,15 @@ def test_unlink_linked_session(self, tmp_path): dataset.add_linked_session(session) # Verify session is linked - refs, session_ids = dataset.session_list() + refs, session_ids, *_ = dataset.session_list() assert len(session_ids) == 1 assert session_ids[0] == session.id() # Unlink - dataset.unlink_session(session.id()) + dataset.unlink_session(session.id(), are_you_sure=True) # Verify session is gone from dataset - refs_after, ids_after = dataset.session_list() + refs_after, ids_after, *_ = dataset.session_list() assert len(ids_after) == 0, "Session list should be empty after unlink" # Session files should still exist @@ -348,53 +344,10 @@ def test_unlink_linked_session(self, tmp_path): session.path / ".ndi" ).exists(), "Session .ndi directory should still exist after unlink" - def test_unlink_with_remove_documents(self, tmp_path): - """Unlink with remove_documents=True removes session docs from dataset. - - MATLAB equivalent: testUnlinkSession.testUnlinkAndDeleteSession - (MATLAB uses 'AlsoDeleteSessionAfterUnlinking'; Python uses remove_documents) - - NOTE: Python does not delete the original session directory. - It only removes documents from the dataset's internal database. - """ - # Create session with docs - session_dir = tmp_path / "sess_rm" - session_dir.mkdir() - session = DirSession("exp_rm", session_dir) - for i in range(1, 4): - _add_doc_with_file(session, i) - - # Create dataset, ingest session - ds_dir = tmp_path / "ds_rm" - ds_dir.mkdir() - dataset = Dataset(ds_dir, "ds_rm") - dataset.add_ingested_session(session) - - session_id = session.id() - - # Verify docs were ingested - q = Query("base.session_id") == session_id - docs_before = dataset.database_search(q) - assert len(docs_before) > 0 - - # Unlink with document removal - dataset.unlink_session(session_id, remove_documents=True) - - # Verify session removed from list - refs_after, ids_after = dataset.session_list() - assert len(ids_after) == 0, "Session list should be empty" - - # Verify documents removed from dataset - docs_after = dataset.database_search(q) - assert len(docs_after) == 0, "Session documents should be removed" - - def test_unlink_ingested_session(self, tmp_path): - """Unlinking an ingested session (without remove_documents). + def test_unlink_ingested_session_error(self, tmp_path): + """Unlinking an ingested session raises ValueError. MATLAB equivalent: testUnlinkSession.testUnlinkIngestedSessionError - NOTE: MATLAB raises an error for this case. - Python allows it — the session_in_a_dataset doc is removed, - but ingested documents remain in the database. """ # Create session session_dir = tmp_path / "sess_ing" @@ -410,29 +363,38 @@ def test_unlink_ingested_session(self, tmp_path): session_id = session.id() - # In Python, unlink_session works for any session type - dataset.unlink_session(session_id) - - # Verify session removed from list - refs_after, ids_after = dataset.session_list() - assert len(ids_after) == 0 + # Unlinking an ingested session should raise + with pytest.raises(ValueError, match="INGESTED"): + dataset.unlink_session(session_id, are_you_sure=True) def test_unlink_nonexistent_session(self, tmp_path): - """Unlinking a nonexistent session does nothing. + """Unlinking a nonexistent session raises ValueError. - MATLAB equivalent: testUnlinkSession.testUnlinkNotConfirmedError - NOTE: Python doesn't require areYouSure for unlink. + MATLAB equivalent: testUnlinkSession.testUnlinkNotFoundError """ ds_dir = tmp_path / "ds_empty" ds_dir.mkdir() dataset = Dataset(ds_dir, "ds_empty") - # Should not raise - dataset.unlink_session("nonexistent_id") + with pytest.raises(ValueError, match="not found"): + dataset.unlink_session("nonexistent_id", are_you_sure=True) + + def test_unlink_not_confirmed(self, tmp_path): + """Unlinking without are_you_sure raises ValueError. + + MATLAB equivalent: testUnlinkSession.testUnlinkNotConfirmedError + """ + session_dir = tmp_path / "sess_conf" + session_dir.mkdir() + session = DirSession("exp_conf", session_dir) + + ds_dir = tmp_path / "ds_conf" + ds_dir.mkdir() + dataset = Dataset(ds_dir, "ds_conf") + dataset.add_linked_session(session) - # Verify still empty - refs, session_ids = dataset.session_list() - assert len(session_ids) == 0 + with pytest.raises(ValueError, match="are_you_sure"): + dataset.unlink_session(session.id()) # =========================================================================== @@ -479,7 +441,7 @@ def test_open_existing_dataset(self, tmp_path): assert reopened.id() == original_ds_id, "Reopened dataset should have same ID" # 4. Verify session list - refs, session_ids = reopened.session_list() + refs, session_ids, *_ = reopened.session_list() assert len(session_ids) >= 1, "Reopened dataset should have at least 1 session" assert ( original_session_id in session_ids diff --git a/tests/matlab_tests/test_session.py b/tests/matlab_tests/test_session.py index 8b9e054..f913f0f 100644 --- a/tests/matlab_tests/test_session.py +++ b/tests/matlab_tests/test_session.py @@ -105,7 +105,7 @@ def test_standalone_session_not_in_dataset(self, tmp_path): ds_dir.mkdir() dataset = Dataset(ds_dir, "ds_empty") - refs, session_ids = dataset.session_list() + refs, session_ids, *_ = dataset.session_list() assert session.id() not in session_ids, "Standalone session should not be in dataset" def test_ingested_session_in_dataset(self, tmp_path): @@ -132,7 +132,7 @@ def test_ingested_session_in_dataset(self, tmp_path): dataset = Dataset(ds_dir, "ds") dataset.add_ingested_session(session) - refs, session_ids = dataset.session_list() + refs, session_ids, *_ = dataset.session_list() assert session.id() in session_ids, "Ingested session should appear in dataset session list" # Also verify the document was ingested @@ -152,7 +152,7 @@ def test_linked_session_in_dataset(self, tmp_path): dataset = Dataset(ds_dir, "ds_link") dataset.add_linked_session(session) - refs, session_ids = dataset.session_list() + refs, session_ids, *_ = dataset.session_list() assert len(session_ids) == 1 assert session_ids[0] == session.id() diff --git a/tests/test_phase1_gaps.py b/tests/test_phase1_gaps.py index 59346ab..6c68578 100644 --- a/tests/test_phase1_gaps.py +++ b/tests/test_phase1_gaps.py @@ -469,7 +469,7 @@ def test_already_copied(self): session = MagicMock() session.id.return_value = "session_123" dataset = MagicMock() - dataset.session_list.return_value = (["ref1"], ["session_123"]) + dataset.session_list.return_value = (["ref1"], ["session_123"], ["doc_123"], "ds_doc_id") success, msg = copy_session_to_dataset(session, dataset) assert not success @@ -485,7 +485,7 @@ def test_successful_copy(self): session.database_search.return_value = [doc] dataset = MagicMock() - dataset.session_list.return_value = ([], []) + dataset.session_list.return_value = ([], [], [], "") success, msg = copy_session_to_dataset(session, dataset) assert success diff --git a/tests/test_phase8.py b/tests/test_phase8.py index fb3fd3f..01f5f53 100644 --- a/tests/test_phase8.py +++ b/tests/test_phase8.py @@ -409,7 +409,7 @@ def test_add_linked_session(self, temp_dir, session): ds = Dataset(temp_dir / "dataset1", "Test") ds.add_linked_session(session) - refs, session_ids = ds.session_list() + refs, session_ids, *_ = ds.session_list() assert len(session_ids) == 1 assert session_ids[0] == session.id() @@ -419,16 +419,18 @@ def test_add_multiple_sessions(self, temp_dir, session, session2): ds.add_linked_session(session) ds.add_linked_session(session2) - refs, session_ids = ds.session_list() + refs, session_ids, *_ = ds.session_list() assert len(session_ids) == 2 def test_add_duplicate_session(self, temp_dir, session): - """Test that adding same session twice doesn't duplicate.""" + """Test that adding same session twice raises ValueError.""" ds = Dataset(temp_dir / "dataset1", "Test") ds.add_linked_session(session) - ds.add_linked_session(session) - refs, session_ids = ds.session_list() + with pytest.raises(ValueError, match="already part of"): + ds.add_linked_session(session) + + refs, session_ids, *_ = ds.session_list() assert len(session_ids) == 1 def test_add_ingested_session(self, temp_dir, session): @@ -441,29 +443,37 @@ def test_add_ingested_session(self, temp_dir, session): ds = Dataset(temp_dir / "dataset1", "Test") ds.add_ingested_session(session) - refs, session_ids = ds.session_list() + refs, session_ids, *_ = ds.session_list() assert len(session_ids) == 1 def test_unlink_session(self, temp_dir, session): """Test unlinking a session.""" ds = Dataset(temp_dir / "dataset1", "Test") ds.add_linked_session(session) - refs, session_ids = ds.session_list() + refs, session_ids, *_ = ds.session_list() assert len(session_ids) == 1 - ds.unlink_session(session.id()) - refs, session_ids = ds.session_list() + ds.unlink_session(session.id(), are_you_sure=True) + refs, session_ids, *_ = ds.session_list() assert len(session_ids) == 0 def test_unlink_nonexistent(self, temp_dir): - """Test unlinking a session that doesn't exist (no error).""" + """Test unlinking a nonexistent session raises ValueError.""" ds = Dataset(temp_dir / "dataset1", "Test") - ds.unlink_session("nonexistent_id") # Should not raise + with pytest.raises(ValueError, match="not found"): + ds.unlink_session("nonexistent_id", are_you_sure=True) + + def test_unlink_requires_confirmation(self, temp_dir, session): + """Test that unlinking requires are_you_sure=True.""" + ds = Dataset(temp_dir / "dataset1", "Test") + ds.add_linked_session(session) + with pytest.raises(ValueError, match="are_you_sure"): + ds.unlink_session(session.id()) def test_session_list_empty(self, temp_dir): """Test session list on empty dataset.""" ds = Dataset(temp_dir / "dataset1", "Test") - refs, session_ids = ds.session_list() + refs, session_ids, *_ = ds.session_list() assert refs == [] assert session_ids == [] @@ -472,7 +482,7 @@ def test_session_list_details(self, temp_dir, session): ds = Dataset(temp_dir / "dataset1", "Test") ds.add_linked_session(session) - refs, session_ids = ds.session_list() + refs, session_ids, *_ = ds.session_list() assert len(session_ids) == 1 assert len(refs) == 1 assert isinstance(refs[0], str) @@ -518,11 +528,11 @@ def test_delete_ingested_session(self, temp_dir, session): """Test deleting an ingested session.""" ds = Dataset(temp_dir / "dataset1", "Test") ds.add_ingested_session(session) - refs, session_ids = ds.session_list() + refs, session_ids, *_ = ds.session_list() assert len(session_ids) == 1 ds.delete_ingested_session(session.id(), are_you_sure=True) - refs, session_ids = ds.session_list() + refs, session_ids, *_ = ds.session_list() assert len(session_ids) == 0 def test_delete_linked_session_raises(self, temp_dir, session): @@ -600,7 +610,7 @@ def test_dataset_with_sessions_and_subjects(self, temp_dir, session, session2): ds.add_linked_session(session2) # Verify - refs, session_ids = ds.session_list() + refs, session_ids, *_ = ds.session_list() assert len(session_ids) == 2 except FileNotFoundError: pytest.skip("Schema not available") From 16edf45cf22f006718abe9dd98d36903091c37d4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Mar 2026 00:04:48 +0000 Subject: [PATCH 6/6] Fix black formatting in dataset and test files https://claude.ai/code/session_01ManyQhMJJszw7Euj7V31Z1 --- src/ndi/dataset/_dataset.py | 23 ++++++----------------- tests/matlab_tests/test_dataset.py | 8 ++++---- 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/src/ndi/dataset/_dataset.py b/src/ndi/dataset/_dataset.py index e55d5d0..06e9cf0 100644 --- a/src/ndi/dataset/_dataset.py +++ b/src/ndi/dataset/_dataset.py @@ -115,8 +115,7 @@ def add_linked_session(self, session: Any) -> Dataset: existing = self._find_session_doc(session.id()) if existing is not None: raise ValueError( - f"Session with id {session.id()} is already part of " - f"dataset {self.id()}." + f"Session with id {session.id()} is already part of " f"dataset {self.id()}." ) # Create session_in_a_dataset document @@ -148,8 +147,7 @@ def add_ingested_session(self, session: Any) -> Dataset: existing = self._find_session_doc(session.id()) if existing is not None: raise ValueError( - f"Session with id {session.id()} is already part of " - f"dataset {self.id()}." + f"Session with id {session.id()} is already part of " f"dataset {self.id()}." ) # Check if session is fully ingested @@ -206,10 +204,7 @@ def unlink_session( doc = self._find_session_doc(session_id) if doc is None: - raise ValueError( - f"Session with ID {session_id} not found in " - f"dataset {self.id()}." - ) + raise ValueError(f"Session with ID {session_id} not found in " f"dataset {self.id()}.") props = doc.document_properties.get("session_in_a_dataset", {}) if not props.get("is_linked", False): @@ -292,16 +287,12 @@ def session_list( # Find the dataset's own session document dataset_session_doc_id = "" - q_ds = Query("").isa("session") & ( - Query("base.session_id") == self.id() - ) + q_ds = Query("").isa("session") & (Query("base.session_id") == self.id()) ds_docs = self._session.database_search(q_ds) if len(ds_docs) == 1: dataset_session_doc_id = ds_docs[0].id elif len(ds_docs) > 1: - raise ValueError( - "More than 1 session document for the dataset session found." - ) + raise ValueError("More than 1 session document for the dataset session found.") return ref_list, id_list, session_doc_ids, dataset_session_doc_id @@ -392,9 +383,7 @@ def delete_ingested_session( doc = self._find_session_doc(session_id) if doc is None: - raise ValueError( - f"Session {session_id} not found in dataset." - ) + raise ValueError(f"Session {session_id} not found in dataset.") props = doc.document_properties.get("session_in_a_dataset", {}) if props.get("is_linked", False): diff --git a/tests/matlab_tests/test_dataset.py b/tests/matlab_tests/test_dataset.py index 297217f..484983f 100644 --- a/tests/matlab_tests/test_dataset.py +++ b/tests/matlab_tests/test_dataset.py @@ -180,9 +180,7 @@ def test_session_list_outputs(self, build_dataset): assert refs[0] == "exp_demo", "Session reference should match expected value" # 2. Verify session_id - assert ( - session_ids[0] == session.id() - ), "Session ID should match the ingested session ID" + assert session_ids[0] == session.id(), "Session ID should match the ingested session ID" # 3. Verify the session_in_a_dataset document exists and is correct q = Query("").isa("session_in_a_dataset") @@ -261,7 +259,9 @@ def test_delete_not_confirmed(self, build_dataset): # Verify session still exists refs, session_ids, *_ = dataset.session_list() - assert session_id in session_ids, "Session ID should still be in dataset after failed delete" + assert ( + session_id in session_ids + ), "Session ID should still be in dataset after failed delete" def test_delete_linked_session_error(self, build_dataset, tmp_path): """Deleting a linked session raises ValueError.