From e59802f233c4cb4f325f70a95ab9160dff136e0e Mon Sep 17 00:00:00 2001 From: solarsys Date: Sun, 27 Sep 2026 11:10:20 -0700 Subject: [PATCH] Keep missing Event timestamps as None; make Event copyable and picklable Two bugs in pyhealth.data.Event: 1. Event.__init__ replaced a missing timestamp with datetime.now(). Tables without a time column (MIMIC-III/IV `patients`, the eICU tables) have a null timestamp, so every Event built from them via get_events() got an invented, ever-changing time that disagreed with the DataFrame view (`return_df=True` shows null). Because MIMIC dates are shifted to the 2100s, the invented time also sorts before every real event, silently passing time-window filters. The timestamp is now None when missing. This is a behaviour change for code that relied on the now() default. 2. Event.__getattr__ read self.attr_dict. copy/pickle create the object without __init__ and probe for dunder methods before attr_dict exists, so pickle.loads, copy.copy and copy.deepcopy recursed until RecursionError. pickle.dumps succeeded, so a task sample holding an Event was written fine and crashed when read back. __getattr__ now rejects dunder names and reads attr_dict through __dict__. - tests/core/test_event.py: missing/explicit timestamps, from_dict and get_events agreeing with the DataFrame, pickle/copy/deepcopy round trips, attribute access, and an Event inside a sample dataset. - docs: document timeless events and copy/pickle support. - examples/event_timestamps_mimic3demo.py on the bundled demo data. Co-Authored-By: Claude Opus 5.5 --- docs/api/data/pyhealth.data.Event.rst | 21 +++++ examples/event_timestamps_mimic3demo.py | 52 ++++++++++++ pyhealth/data/data.py | 39 ++++++--- tests/core/test_event.py | 100 ++++++++++++++++++++++++ 4 files changed, 199 insertions(+), 13 deletions(-) create mode 100644 examples/event_timestamps_mimic3demo.py create mode 100644 tests/core/test_event.py diff --git a/docs/api/data/pyhealth.data.Event.rst b/docs/api/data/pyhealth.data.Event.rst index 04c91fe52..d6f1f4987 100644 --- a/docs/api/data/pyhealth.data.Event.rst +++ b/docs/api/data/pyhealth.data.Event.rst @@ -4,6 +4,27 @@ One basic data structure in the package. It is a simple container for a single event. It contains all necessary attributes for supporting various healthcare tasks. +Events without a time +--------------------- + +Some tables have no timestamp column, for example MIMIC-III/IV ``patients`` +(demographics) and the eICU tables. Their events have ``timestamp`` set to +``None``, matching the ``null`` value in ``patient.get_events(..., return_df=True)``. +Check for it before comparing or sorting by time: + +.. code-block:: python + + timed = [e for e in patient.get_events() if e.timestamp is not None] + +.. note:: + + In PyHealth 2.0.2 and earlier, a missing timestamp was replaced with + ``datetime.now()``, which produced a different, invented time on every call. + Pass ``timestamp=`` explicitly if you create events by hand and need a time. + +``Event`` objects can be copied (``copy.copy`` / ``copy.deepcopy``) and pickled, +so they can be stored in task samples and sent to worker processes. + .. autoclass:: pyhealth.data.Event :members: :undoc-members: diff --git a/examples/event_timestamps_mimic3demo.py b/examples/event_timestamps_mimic3demo.py new file mode 100644 index 000000000..59304bb88 --- /dev/null +++ b/examples/event_timestamps_mimic3demo.py @@ -0,0 +1,52 @@ +"""Working with Event timestamps, including events that have no time. + +Some tables have no timestamp column (e.g. MIMIC-III ``patients``). Their +events have ``timestamp=None``, the same ``null`` shown by +``get_events(..., return_df=True)``. This script shows how to read them, how to +keep only timed events before comparing times, and that events can be pickled +(e.g. stored in task samples). + +Uses the small MIMIC-III demo bundled in ``test-resources/``; no download. + +Usage (from the repository root): + python examples/event_timestamps_mimic3demo.py +""" + +import pickle +from pathlib import Path + +from pyhealth.datasets import MIMIC3Dataset + +DEMO_ROOT = ( + Path(__file__).resolve().parent.parent / "test-resources" / "core" / "mimic3demo" +) + + +def main(): + dataset = MIMIC3Dataset(root=str(DEMO_ROOT), tables=["diagnoses_icd"]) + patient = dataset.get_patient("10006") + + # Demographics have no time: the Event agrees with the DataFrame view. + demographics = patient.get_events(event_type="patients")[0] + df = patient.get_events(event_type="patients", return_df=True) + print("patients event timestamp:", demographics.timestamp) + print("patients row timestamp: ", df["timestamp"].to_list()[0]) + print("gender:", demographics.gender) + + # Keep only timed events before comparing or sorting by time. + admission = patient.get_events(event_type="admissions")[0] + events = patient.get_events() + timed = [e for e in events if e.timestamp is not None] + before_admission = [e for e in timed if e.timestamp <= admission.timestamp] + print( + f"{len(events)} events, {len(timed)} with a time, " + f"{len(before_admission)} at or before the first admission" + ) + + # Events survive pickling, so tasks can put them in samples. + restored = pickle.loads(pickle.dumps(admission)) + print("pickle round trip equal:", restored == admission) + + +if __name__ == "__main__": + main() diff --git a/pyhealth/data/data.py b/pyhealth/data/data.py index 14b1b526c..abc1b6e6e 100644 --- a/pyhealth/data/data.py +++ b/pyhealth/data/data.py @@ -14,21 +14,34 @@ class Event: Attributes: event_type (str): Type of the clinical event (e.g., 'medication', 'diagnosis') - timestamp (datetime): When the event occurred + timestamp (Optional[datetime]): When the event occurred, or ``None`` if + the event has no time (e.g. rows of a demographics table such as + MIMIC's ``patients``) attr_dict (Mapping[str, any]): Dictionary containing event-specific attributes + + Examples: + >>> from datetime import datetime + >>> from pyhealth.data import Event + >>> event = Event("admissions", timestamp=datetime(2164, 10, 23), hadm_id="1") + >>> event.hadm_id + '1' + >>> Event("patients", gender="F").timestamp is None + True """ event_type: str - timestamp: datetime + timestamp: datetime | None attr_dict: Mapping[str, any] = field(default_factory=dict) - def __init__(self, event_type: str, timestamp: datetime = None, **kwargs): + def __init__( + self, event_type: str, timestamp: datetime | None = None, **kwargs + ): """Initialize an Event instance. Args: event_type (str): Type of the clinical event - timestamp (datetime, optional): When the event occurred. - If not provided, current time will be used. + timestamp (datetime, optional): When the event occurred. Defaults to + ``None``, meaning the event has no time. **kwargs: Additional attributes to store in attr_dict """ # Create a mutable copy of kwargs to manipulate @@ -40,10 +53,6 @@ def __init__(self, event_type: str, timestamp: datetime = None, **kwargs): # Merge with remaining kwargs, with kwargs taking precedence attr_dict = {**existing_attr_dict, **attr_dict} - # Set timestamp to current time if not provided - if timestamp is None: - timestamp = datetime.now() - # Use object.__setattr__ since the dataclass is frozen object.__setattr__(self, "event_type", event_type) object.__setattr__(self, "timestamp", timestamp) @@ -107,10 +116,14 @@ def __getattr__(self, key: str) -> any: Raises: AttributeError: If the attribute does not exist. """ - if key == "timestamp" or key == "event_type": - return getattr(self, key) - if key in self.attr_dict: - return self.attr_dict[key] + # Only called when normal lookup fails. copy and pickle build the object + # without __init__ and probe for dunder methods before attr_dict exists, + # so never touch self.attr_dict here (it would recurse forever). + if key.startswith("__") and key.endswith("__"): + raise AttributeError(key) + attr_dict = self.__dict__.get("attr_dict") + if attr_dict is not None and key in attr_dict: + return attr_dict[key] raise AttributeError(f"'Event' object has no attribute '{key}'") diff --git a/tests/core/test_event.py b/tests/core/test_event.py new file mode 100644 index 000000000..f0e01c5a3 --- /dev/null +++ b/tests/core/test_event.py @@ -0,0 +1,100 @@ +"""Tests for pyhealth.data.Event: missing timestamps and copy/pickle support.""" + +import copy +import pickle +import unittest +from datetime import datetime + +import polars as pl + +from pyhealth.data import Event, Patient +from pyhealth.datasets import create_sample_dataset + + +def _patient() -> Patient: + # A timeless demographics row (like MIMIC's `patients` table) and a timed row. + df = pl.DataFrame( + { + "patient_id": ["p1", "p1"], + "event_type": ["patients", "admissions"], + "timestamp": [None, datetime(2164, 10, 23, 21, 9)], + "patients/gender": ["F", None], + "admissions/hadm_id": [None, "142345"], + }, + schema_overrides={"timestamp": pl.Datetime("ms")}, + ) + return Patient(patient_id="p1", data_source=df) + + +class TestEventTimestamp(unittest.TestCase): + def test_missing_timestamp_is_none(self): + self.assertIsNone(Event("note").timestamp) + self.assertIsNone(Event("note", timestamp=None).timestamp) + + def test_explicit_timestamp_is_kept(self): + ts = datetime(2020, 1, 2, 3, 4) + self.assertEqual(Event("note", timestamp=ts).timestamp, ts) + + def test_from_dict_keeps_null_timestamp(self): + event = Event.from_dict( + {"event_type": "patients", "timestamp": None, "patients/gender": "F"} + ) + self.assertIsNone(event.timestamp) + self.assertEqual(event.gender, "F") + + def test_get_events_matches_dataframe_and_is_stable(self): + patient = _patient() + df_ts = patient.get_events("patients", return_df=True)["timestamp"].to_list() + first = patient.get_events("patients")[0].timestamp + second = patient.get_events("patients")[0].timestamp + self.assertEqual(df_ts, [None]) + self.assertIsNone(first) + self.assertIsNone(second) + admission = patient.get_events("admissions")[0] + self.assertEqual(admission.timestamp, datetime(2164, 10, 23, 21, 9)) + + +class TestEventCopyAndPickle(unittest.TestCase): + def setUp(self): + self.event = Event( + "admissions", timestamp=datetime(2164, 10, 23), hadm_id="142345" + ) + + def _assert_same(self, other: Event): + self.assertIsNot(other, self.event) + self.assertEqual(other, self.event) + self.assertEqual(other.hadm_id, "142345") + self.assertEqual(other.timestamp, datetime(2164, 10, 23)) + + def test_pickle_round_trip(self): + self._assert_same(pickle.loads(pickle.dumps(self.event))) + + def test_copy_and_deepcopy(self): + self._assert_same(copy.copy(self.event)) + clone = copy.deepcopy(self.event) + self._assert_same(clone) + self.assertIsNot(clone.attr_dict, self.event.attr_dict) + + def test_attribute_access(self): + self.assertEqual(self.event["hadm_id"], "142345") + self.assertIn("hadm_id", self.event) + with self.assertRaises(AttributeError): + _ = self.event.not_a_field + self.assertFalse(hasattr(self.event, "not_a_field")) + + def test_event_inside_a_sample(self): + samples = [ + {"patient_id": f"p{i}", "codes": ["a"], "admission": self.event, "label": i % 2} + for i in range(4) + ] + dataset = create_sample_dataset( + samples=samples, + input_schema={"codes": "sequence", "admission": "raw"}, + output_schema={"label": "binary"}, + dataset_name="test_event", + ) + self._assert_same(dataset[0]["admission"]) + + +if __name__ == "__main__": + unittest.main()