1681: added @cached decorator to get_dataset (#1687)

alexfurmenkov · SFJohnson24 · web-flow · commit 21ac8e66bebe · 2026-04-30T16:57:26.000-04:00
* added cached_dataset decorator to get_dataset

* added per-builder dataset caching on rule validation.

* cache tests

* fix for tests -- RulesEngine cache is cleared before each test

* added fields for caching to BaseDatasetBuilder

* remove comment

* removed unnecessary kwargs pop call

* test for ignored kwargs in split dataset build

* added cached decorator to JSONataDatasetBuilder

* removed kwargs from get_dataset method

* fix test

* removed params and unused drop_duplicates argument from get metadata

* remove kwargs

* test

---------

Co-authored-by: Samuel Johnson &lt;96841389+SFJohnson24@users.noreply.github.com&gt;
Co-authored-by: Samuel Johnson &lt;sfjohnson24@gmail.com&gt;
diff --git a/cdisc_rules_engine/dataset_builders/base_dataset_builder.py b/cdisc_rules_engine/dataset_builders/base_dataset_builder.py
@@ -5,6 +5,7 @@
 from cdisc_rules_engine.services.define_xml.define_xml_reader_factory import (
     DefineXMLReaderFactory,
 )
+from cdisc_rules_engine.utilities.decorators import cached
 from cdisc_rules_engine.utilities.sdtm_utilities import get_corresponding_datasets
 from cdisc_rules_engine.utilities.sdtm_utilities import (
     tag_source,
@@ -34,6 +35,7 @@ def __init__(
     ):
         self.data_service = data_service
         self.cache = cache_service
+        self.cache_service = cache_service
         self.data_processor = data_processor
         self.rule_processor = rule_processor
         self.dataset_metadata = dataset_metadata
@@ -44,6 +46,14 @@ def __init__(
         self.standard_substandard = standard_substandard
         self.library_metadata = library_metadata
         self.dataset_implementation = self.data_service.dataset_implementation
+        if isinstance(dataset_metadata, SDTMDatasetMetadata):
+            self.domain = (
+                f"SUPP{dataset_metadata.rdomain}"
+                if dataset_metadata.rdomain
+                else dataset_metadata.domain
+            )
+            self.dataset_name = dataset_metadata.name
+        self.name = self.__class__.__name__
 
     @abstractmethod
     def build(self) -> DatasetInterface:
@@ -67,7 +77,8 @@ def build_split_datasets(self, dataset_name: str, **kwargs) -> DatasetInterface:
         finally:
             self.dataset_metadata = original_dataset_metadata
 
-    def get_dataset(self, **kwargs):
+    @cached("get_dataset")
+    def get_dataset(self):
         # If validating dataset content, ensure split datasets are handled.
         if self.dataset_metadata.is_split:
             # Handle split datasets for content checks.
@@ -77,15 +88,14 @@ def get_dataset(self, **kwargs):
                 datasets_metadata=get_corresponding_datasets(
                     self.data_service.get_datasets(), self.dataset_metadata
                 ),
-                **kwargs,
             )
         else:
             # single dataset. the most common case
             dataset: DatasetInterface = self.build()
             dataset = tag_source(dataset, self.dataset_metadata)
         return dataset
 
-    def get_dataset_contents(self, **kwargs):
+    def get_dataset_contents(self):
         # If validating dataset content, ensure split datasets are handled.
         if self.dataset_metadata.is_split:
             # Handle split datasets for content checks.
@@ -95,7 +105,6 @@ def get_dataset_contents(self, **kwargs):
                 datasets_metadata=get_corresponding_datasets(
                     self.data_service.get_datasets(), self.dataset_metadata
                 ),
-                **kwargs,
             )
         else:
             # single dataset. the most common case
diff --git a/cdisc_rules_engine/dataset_builders/contents_dataset_builder.py b/cdisc_rules_engine/dataset_builders/contents_dataset_builder.py
@@ -17,8 +17,8 @@ def build_split_datasets(self, dataset_name, **kwargs):
         """
         return self.data_service.get_dataset(dataset_name=dataset_name)
 
-    def get_dataset(self, **kwargs):
-        dataset = super().get_dataset(**kwargs)
+    def get_dataset(self):
+        dataset = super().get_dataset()
         length = sum(
             [
                 dataset.record_count
diff --git a/cdisc_rules_engine/dataset_builders/domain_list_dataset_builder.py b/cdisc_rules_engine/dataset_builders/domain_list_dataset_builder.py
@@ -14,6 +14,5 @@ def build(self):
         """
 
         return self.dataset_implementation.from_records(
-            {ds.unsplit_name: ds.filename for ds in self.data_service.get_datasets()},
-            index=[0],
+            [{ds.unsplit_name: ds.filename for ds in self.data_service.get_datasets()}]
         )
diff --git a/cdisc_rules_engine/dataset_builders/json_schema_check_dataset_builder.py b/cdisc_rules_engine/dataset_builders/json_schema_check_dataset_builder.py
@@ -46,7 +46,7 @@ def _get_cached_dataset(self) -> dict[str, list[str]]:
 
         return errlist
 
-    def get_dataset(self, **kwargs) -> DatasetInterface:
+    def get_dataset(self) -> DatasetInterface:
         dataset = self._get_cached_dataset()
         records = [
             {key: dataset[key][i] for key in dataset}
@@ -56,10 +56,10 @@ def get_dataset(self, **kwargs) -> DatasetInterface:
             row for row in records if row["dataset"] == self.dataset_metadata.name
         ]
         if filtered:
-            result = self.dataset_implementation.from_records(filtered, **kwargs)
+            result = self.dataset_implementation.from_records(filtered)
         else:
             empty_row = {key: "" for key in self.dataset_template.keys()}
-            result = self.dataset_implementation.from_records([empty_row], **kwargs)
+            result = self.dataset_implementation.from_records([empty_row])
         return tag_source(result, self.dataset_metadata)
 
     def list_errors(self, tree: exceptions.ErrorTree, errlist: dict[str, list]):
diff --git a/cdisc_rules_engine/dataset_builders/jsonata_dataset_builder.py b/cdisc_rules_engine/dataset_builders/jsonata_dataset_builder.py
@@ -1,5 +1,6 @@
 from json import load
 from cdisc_rules_engine.dataset_builders.base_dataset_builder import BaseDatasetBuilder
+from cdisc_rules_engine.utilities.decorators import cached
 
 
 def add_json_pointer_paths(node, path=""):
@@ -19,7 +20,8 @@ def add_json_pointer_paths(node, path=""):
 
 class JSONataDatasetBuilder(BaseDatasetBuilder):
 
-    def get_dataset(self, **kwargs):
+    @cached("get_dataset")
+    def get_dataset(self):
         if not self.dataset_metadata.full_path:
             return None
         with self.data_service.read_data(self.dataset_metadata.full_path) as fp:
diff --git a/cdisc_rules_engine/dataset_builders/variables_metadata_dataset_builder.py b/cdisc_rules_engine/dataset_builders/variables_metadata_dataset_builder.py
@@ -17,8 +17,7 @@ def build(self):
         """
         # Get basic variable metadata
         variables_metadata = self.data_service.get_variables_metadata(
-            dataset_name=self.dataset_metadata.name,
-            drop_duplicates=True,
+            dataset_name=self.dataset_metadata.name
         )
 
         # Check if the rule requires variable_max_size
diff --git a/cdisc_rules_engine/dataset_builders/variables_metadata_values_dataset_builder.py b/cdisc_rules_engine/dataset_builders/variables_metadata_values_dataset_builder.py
@@ -22,8 +22,7 @@ def build(self):
         """
         data_contents_long_df = super().build()
         variable_metadata = self.data_service.get_variables_metadata(
-            dataset_name=self.dataset_metadata.name,
-            drop_duplicates=True,
+            dataset_name=self.dataset_metadata.name
         )
         merged_df = data_contents_long_df.merge(
             variable_metadata._data, how="left", on="variable_name"
diff --git a/cdisc_rules_engine/dataset_builders/variables_metadata_with_define_and_library_dataset_builder.py b/cdisc_rules_engine/dataset_builders/variables_metadata_with_define_and_library_dataset_builder.py
@@ -42,8 +42,7 @@ def build(self):
         """
         variable_metadata: List[dict] = self.get_define_xml_variables_metadata()
         content_metadata: DatasetInterface = self.data_service.get_variables_metadata(
-            dataset_name=self.dataset_metadata.name,
-            drop_duplicates=True,
+            dataset_name=self.dataset_metadata.name
         )
         define_metadata: DatasetInterface = self.dataset_implementation.from_records(
             variable_metadata
diff --git a/cdisc_rules_engine/dataset_builders/variables_metadata_with_define_dataset_builder.py b/cdisc_rules_engine/dataset_builders/variables_metadata_with_define_dataset_builder.py
@@ -36,8 +36,7 @@ def build(self):
         variable_metadata: List[dict] = self.get_define_xml_variables_metadata()
         # get dataset metadata and execute the rule
         content_metadata: DatasetInterface = self.data_service.get_variables_metadata(
-            dataset_name=self.dataset_metadata.name,
-            drop_duplicates=True,
+            dataset_name=self.dataset_metadata.name
         )
         define_metadata: DatasetInterface = self.dataset_implementation.from_records(
             variable_metadata
diff --git a/cdisc_rules_engine/dataset_builders/variables_metadata_with_library_metadata.py b/cdisc_rules_engine/dataset_builders/variables_metadata_with_library_metadata.py
@@ -26,8 +26,7 @@ def build(self):
         # get dataset metadata and execute the rule
         content_variables_metadata: DatasetInterface = (
             self.data_service.get_variables_metadata(
-                dataset_name=self.dataset_metadata.name,
-                drop_duplicates=True,
+                dataset_name=self.dataset_metadata.name
             )
         )
         dataset_contents = self.get_dataset_contents()
diff --git a/cdisc_rules_engine/interfaces/data_service_interface.py b/cdisc_rules_engine/interfaces/data_service_interface.py
@@ -52,7 +52,7 @@ def get_raw_dataset_metadata(
         """
 
     @abstractmethod
-    def get_variables_metadata(self, dataset_name: str, **params) -> DatasetInterface:
+    def get_variables_metadata(self, dataset_name: str) -> DatasetInterface:
         """
         Gets variables metadata of a dataset.
         """
@@ -71,7 +71,6 @@ def concat_split_datasets(
         self,
         func_to_call: Callable,
         datasets_metadata: Iterable[DatasetMetadata],
-        **kwargs,
     ):
         """
         Accepts a list of split dataset filenames,
diff --git a/cdisc_rules_engine/models/dataset/dask_dataset.py b/cdisc_rules_engine/models/dataset/dask_dataset.py
@@ -117,8 +117,8 @@ def from_dict(cls, data: dict, **kwargs):
         return cls(dataframe)
 
     @classmethod
-    def from_records(cls, data: List[dict], **kwargs):
-        data = pd.DataFrame.from_records(data, **kwargs)
+    def from_records(cls, data: List[dict]):
+        data = pd.DataFrame.from_records(data)
         dataframe = dd.from_pandas(data, npartitions=DEFAULT_NUM_PARTITIONS)
         return cls(dataframe)
 
diff --git a/cdisc_rules_engine/models/dataset/dataset_interface.py b/cdisc_rules_engine/models/dataset/dataset_interface.py
@@ -33,7 +33,7 @@ def from_dict(cls, data: dict, **kwargs):
 
     @classmethod
     @abstractmethod
-    def from_records(cls, data: List[dict], **kwargs):
+    def from_records(cls, data: List[dict]):
         """
         Create the underlying dataset from provided list of records
         """
diff --git a/cdisc_rules_engine/models/dataset/pandas_dataset.py b/cdisc_rules_engine/models/dataset/pandas_dataset.py
@@ -56,8 +56,8 @@ def from_dict(cls, data: dict, **kwargs):
         return cls(dataframe)
 
     @classmethod
-    def from_records(cls, data: List[dict], **kwargs):
-        dataframe = pd.DataFrame.from_records(data, **kwargs)
+    def from_records(cls, data: List[dict]):
+        dataframe = pd.DataFrame.from_records(data)
         return cls(dataframe)
 
     def __getitem__(
diff --git a/cdisc_rules_engine/services/data_services/base_data_service.py b/cdisc_rules_engine/services/data_services/base_data_service.py
@@ -140,7 +140,6 @@ def concat_split_datasets(
         self,
         func_to_call: Callable,
         datasets_metadata: Iterable[DatasetMetadata],
-        **kwargs,
     ) -> DatasetInterface:
         """
         Accepts a list of split dataset filenames, asynchronously downloads
@@ -149,24 +148,19 @@ def concat_split_datasets(
         func_to_call must accept dataset_name and kwargs
         as input parameters and return pandas DataFrame.
         """
-        # pop drop_duplicates param at the beginning to avoid passing it to func_to_call
-        drop_duplicates: bool = kwargs.pop("drop_duplicates", False)
 
         # download datasets asynchronously
         datasets: Iterator[DatasetInterface] = self._async_get_datasets(
             func_to_call,
             dataset_names=[
                 dataset_metadata.name for dataset_metadata in datasets_metadata
             ],
-            **kwargs,
         )
         full_dataset = self.dataset_implementation()
         for dataset, dataset_metadata in zip(datasets, datasets_metadata):
             tagged_dataset = tag_source(dataset, dataset_metadata)
             full_dataset = full_dataset.concat(tagged_dataset, ignore_index=True)
 
-        if drop_duplicates:
-            full_dataset = full_dataset.drop_duplicates()
         return full_dataset
 
     def check_filepath(self, dataset_names: List[str]) -> List:
diff --git a/cdisc_rules_engine/services/data_services/dummy_data_service.py b/cdisc_rules_engine/services/data_services/dummy_data_service.py
@@ -81,7 +81,7 @@ def _initialize_datasets_metadata(self, **kwargs) -> dict[str, SDTMDatasetMetada
             for dataset in self.data
         }
 
-    def get_variables_metadata(self, dataset_name: str, **params) -> PandasDataset:
+    def get_variables_metadata(self, dataset_name: str) -> PandasDataset:
         metadata_to_return = {
             "variable_name": [],
             "variable_order_number": [],
diff --git a/cdisc_rules_engine/services/data_services/excel_data_service.py b/cdisc_rules_engine/services/data_services/excel_data_service.py
@@ -202,7 +202,7 @@ def _initialize_datasets_metadata(self, **kwargs) -> dict[str, SDTMDatasetMetada
         return result
 
     @cached_dataset(DatasetTypes.VARIABLES_METADATA.value)
-    def get_variables_metadata(self, dataset_name: str, **params) -> DatasetInterface:
+    def get_variables_metadata(self, dataset_name: str) -> DatasetInterface:
         """
         Gets dataset from blob storage and returns metadata of a certain variable.
         """
diff --git a/cdisc_rules_engine/services/data_services/local_data_service.py b/cdisc_rules_engine/services/data_services/local_data_service.py
@@ -150,7 +150,7 @@ def get_dataset(self, dataset_name: str, **params) -> DatasetInterface:
         return df
 
     @cached_dataset(DatasetTypes.VARIABLES_METADATA.value)
-    def get_variables_metadata(self, dataset_name: str, **params) -> DatasetInterface:
+    def get_variables_metadata(self, dataset_name: str) -> DatasetInterface:
         """
         Gets dataset from blob storage and returns metadata of a certain variable.
         """
diff --git a/cdisc_rules_engine/services/data_services/usdm_data_service.py b/cdisc_rules_engine/services/data_services/usdm_data_service.py
@@ -158,7 +158,7 @@ def _initialize_datasets_metadata(self, **kwargs) -> dict[str, SDTMDatasetMetada
         return result
 
     @cached_dataset(DatasetTypes.VARIABLES_METADATA.value)
-    def get_variables_metadata(self, dataset_name: str, **params) -> DatasetInterface:
+    def get_variables_metadata(self, dataset_name: str) -> DatasetInterface:
         """
         Gets dataset from blob storage and returns metadata of a certain variable.
         """
diff --git a/tests/unit/test_dataset_builders/test_jsonata_dataset_bilder.py b/tests/unit/test_dataset_builders/test_jsonata_dataset_bilder.py
@@ -0,0 +1,50 @@
+from unittest.mock import MagicMock, mock_open, patch
+
+from cdisc_rules_engine.dataset_builders.jsonata_dataset_builder import (
+    JSONataDatasetBuilder,
+)
+
+
+def test_get_dataset_uses_cache():
+    metadata = MagicMock()
+    metadata.full_path = "fake_path.json"
+
+    data_service = MagicMock()
+    data_service.read_data.return_value.__enter__.return_value = mock_open(
+        read_data='{"a": {"b": 1}}'
+    )()
+
+    cache_service = MagicMock()
+    cache_service.get.side_effect = [None, {"cached": True}]
+
+    builder = JSONataDatasetBuilder(
+        dataset_metadata=metadata,
+        data_service=data_service,
+        rule={},
+        cache_service=cache_service,
+        rule_processor=MagicMock(),
+        data_processor=MagicMock(),
+        define_xml_path=None,
+        standard="USDM",
+        standard_version="4.0",
+        standard_substandard=None,
+    )
+    builder.cache_service = cache_service
+
+    with patch(
+        "cdisc_rules_engine.dataset_builders.jsonata_dataset_builder.load",
+        return_value={"a": {"b": 1}},
+    ) as mock_load:
+
+        result1 = builder.get_dataset()
+        result2 = builder.get_dataset()
+
+        assert result1 == {"_path": "", "a": {"_path": "/a", "b": 1}}
+
+        # second call gets from cache
+        assert result2 == {"cached": True}
+
+        assert mock_load.call_count == 1
+
+        assert cache_service.get.call_count == 2
+        assert cache_service.add.call_count == 1
diff --git a/tests/unit/test_rules_engine.py b/tests/unit/test_rules_engine.py
@@ -26,6 +26,11 @@
 from cdisc_rules_engine.models.dataset import PandasDataset
 
 
+@pytest.fixture(autouse=True)
+def clear_lru_cache():
+    RulesEngine(standard="sdtmig").cache.clear_all()
+
+
 def test_get_schema():
     schema = RulesEngine().get_schema()
     assert "variables" in schema
@@ -1075,9 +1080,15 @@ def test_rule_with_domain_prefix_replacement(
                 {
                     "domain": "AE",
                     "dataset": "AE",
-                    "errors": [],
-                    "executionStatus": ExecutionStatus.SUCCESS.value,
-                    "message": None,
+                    "errors": [
+                        {
+                            "dataset": "AE",
+                            "error": "Empty dataset",
+                            "message": "Dataset skipped - Dataset is empty after preprocessing and operations. rule id=TEST1, dataset=AE",
+                        }
+                    ],
+                    "executionStatus": ExecutionStatus.SKIPPED.value,
+                    "message": "Dataset skipped - Dataset is empty after preprocessing and operations. rule id=TEST1, dataset=AE",
                     "variables": [],
                 }
             ],
diff --git a/tests/unit/test_services/test_data_service/test_data_service.py b/tests/unit/test_services/test_data_service/test_data_service.py
diff --git a/tests/unit/test_usdm_data.py b/tests/unit/test_usdm_data.py

Original file line number	Diff line number	Diff line change
`@@ -14,6 +14,5 @@ def build(self):`
`14`	`14`	`"""`
`15`	`15`
`16`	`16`	`return self.dataset_implementation.from_records(`
`17`		`- {ds.unsplit_name: ds.filename for ds in self.data_service.get_datasets()},`
`18`		`- index=[0],`
	`17`	`+ [{ds.unsplit_name: ds.filename for ds in self.data_service.get_datasets()}]`
`19`	`18`	`)`
Original file line number	Diff line number	Diff line change
`@@ -17,8 +17,7 @@ def build(self):`
`17`	`17`	`"""`
`18`	`18`	`# Get basic variable metadata`
`19`	`19`	`variables_metadata = self.data_service.get_variables_metadata(`
`20`		`- dataset_name=self.dataset_metadata.name,`
`21`		`- drop_duplicates=True,`
	`20`	`+ dataset_name=self.dataset_metadata.name`
`22`	`21`	`)`
`23`	`22`
`24`	`23`	`# Check if the rule requires variable_max_size`
Original file line number	Diff line number	Diff line change
`@@ -26,8 +26,7 @@ def build(self):`
`26`	`26`	`# get dataset metadata and execute the rule`
`27`	`27`	`content_variables_metadata: DatasetInterface = (`
`28`	`28`	`self.data_service.get_variables_metadata(`
`29`		`- dataset_name=self.dataset_metadata.name,`
`30`		`- drop_duplicates=True,`
	`29`	`+ dataset_name=self.dataset_metadata.name`
`31`	`30`	`)`
`32`	`31`	`)`
`33`	`32`	`dataset_contents = self.get_dataset_contents()`