Skip to content

Lmdb test fix #876

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Aske-Rosted wants to merge 2 commits into
base: main
Choose a base branch
from
+7 −2
Open

Lmdb test fix #876

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1ec4a62
public close connection and use in test
Aske-Rosted Mar 30, 2026
0dead35
move _close_connetion call to end of post_init
Aske-Rosted Mar 30, 2026
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
6 changes: 5 additions & 1 deletion src/graphnet/data/dataset/lmdb/lmdb_dataset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,9 @@ def _post_init(self) -> None:
if self._pre_computed_representation is None:
# Only check for missing columns if using raw tables
self._remove_missing_columns()
self._close_connection()
if self._pre_computed_representation is not None:
self._identify_missing_truth_labels()
self._close_connection()

def _identify_missing_truth_labels(self) -> None:
"""Identify missing truth labels in the pre-computed representation."""
Expand Down Expand Up @@ -430,3 +430,7 @@ def __getitem__(self, sequential_index: int) -> Any:
data = super().__getitem__(sequential_index)

return data

def close(self) -> None:
"""Close any open LMDB connections."""
self._close_connection()
3 changes: 2 additions & 1 deletion tests/data/test_dataconverters_and_datasets.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,6 @@ def test_sqlite_to_lmdb_converter() -> None:
truth=TRUTH.DEEPCORE,
graph_definition=graph_definition,
)

dataset_from_lmdb_raw = LMDBDataset(path, **opt_raw) # type: ignore
dataset_sqlite = SQLiteDataset(
get_file_path("sqlite"), **opt_raw # type: ignore[arg-type]
Expand All @@ -310,6 +309,7 @@ def test_sqlite_to_lmdb_converter() -> None:
dataset_from_lmdb_raw[ix].x, dataset_sqlite[ix].x
)

dataset_from_lmdb_raw.close() # Close connection
# Test 2: Check that pre-computed representation matches real-time computed
Comment on lines 299 to 313
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dataset_from_lmdb_raw.close() is only reached if all assertions above pass. If an assertion fails, the LMDB environment can remain open and may cause later LMDB tests in the same session to error when creating a new dataset for the same path. Consider wrapping the dataset usage in a try/finally (or using contextlib.closing) so close() is always executed.

Copilot uses AI. Check for mistakes.
# The pre-computed representation field name is the class name
pre_computed_field_name = graph_definition.__class__.__name__
Expand Down Expand Up @@ -361,3 +361,4 @@ def test_sqlite_to_lmdb_converter() -> None:
)
else:
assert precomputed_truth == realtime_truth
dataset_from_lmdb_precomputed.close() # Close connection
Loading