From e667033b1117ee25a9a7ae3c5f007270b8c6a9b7 Mon Sep 17 00:00:00 2001 From: alexfurmenkov Date: Thu, 2 Apr 2026 16:27:58 +0200 Subject: [PATCH 1/4] Add variable_max_size to variables metadata for enhanced data insights --- .../variables_metadata_dataset_builder.py | 40 ++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/cdisc_rules_engine/dataset_builders/variables_metadata_dataset_builder.py b/cdisc_rules_engine/dataset_builders/variables_metadata_dataset_builder.py index cdc8bb285..eb94f74e7 100644 --- a/cdisc_rules_engine/dataset_builders/variables_metadata_dataset_builder.py +++ b/cdisc_rules_engine/dataset_builders/variables_metadata_dataset_builder.py @@ -1,4 +1,5 @@ from cdisc_rules_engine.dataset_builders.base_dataset_builder import BaseDatasetBuilder +import pandas as pd class VariablesMetadataDatasetBuilder(BaseDatasetBuilder): @@ -12,7 +13,44 @@ def build(self): variable_size variable_data_type variable_format + variable_max_size (if needed by the rule) """ - return self.data_service.get_variables_metadata( + # Get basic variable metadata + variables_metadata = self.data_service.get_variables_metadata( dataset_name=self.dataset_path, datasets=self.datasets, drop_duplicates=True ) + + # Check if the rule requires variable_max_size + if ( + self.rule + and self.rule.get("output_variables") + and "variable_max_size" in self.rule["output_variables"] + ): + variables_metadata = self._add_variable_max_size(variables_metadata) + + return variables_metadata + + def _add_variable_max_size(self, variables_metadata): + """ + Add variable_max_size column to the variables metadata. + This column contains the maximum length of actual data for each variable. + """ + # Get the dataset contents + dataset = self.data_service.get_dataset(dataset_name=self.dataset_path) + + # Calculate max size for each variable + max_sizes = {} + for var_name in variables_metadata.data["variable_name"]: + if var_name in dataset.data.columns: + # Convert to string and get max length, ignoring null values + max_length = dataset.data[var_name].dropna().astype(str).str.len().max() + max_sizes[var_name] = max_length if not pd.isna(max_length) else 0 + else: + max_sizes[var_name] = 0 + + # Add the max_size column to metadata + variables_metadata.data["variable_max_size"] = variables_metadata.data[ + "variable_name" + ].map(max_sizes) + + return variables_metadata From 9ab71f133402b7e1275134ba986b40628ed9b5cc Mon Sep 17 00:00:00 2001 From: alexfurmenkov Date: Mon, 6 Apr 2026 18:41:24 +0200 Subject: [PATCH 2/4] Implement checks for variable_max_size in rule processing logic --- .../variables_metadata_dataset_builder.py | 94 ++++++++++++++++++- 1 file changed, 89 insertions(+), 5 deletions(-) diff --git a/cdisc_rules_engine/dataset_builders/variables_metadata_dataset_builder.py b/cdisc_rules_engine/dataset_builders/variables_metadata_dataset_builder.py index eb94f74e7..0c412d37b 100644 --- a/cdisc_rules_engine/dataset_builders/variables_metadata_dataset_builder.py +++ b/cdisc_rules_engine/dataset_builders/variables_metadata_dataset_builder.py @@ -21,15 +21,99 @@ def build(self): ) # Check if the rule requires variable_max_size - if ( - self.rule - and self.rule.get("output_variables") - and "variable_max_size" in self.rule["output_variables"] - ): + if self.rule and self._needs_variable_max_size(): variables_metadata = self._add_variable_max_size(variables_metadata) return variables_metadata + def _needs_variable_max_size(self): + """ + Check if the rule requires variable_max_size by examining: + - output_variables + - operations (operator field) + - conditions (all fields) + """ + return ( + self._check_output_variables_for_variable_max_size() + or self._check_operations_for_variable_max_size() + or self._check_conditions_for_variable_max_size(self.rule.get("conditions")) + ) + + def _check_output_variables_for_variable_max_size(self): + """Check if output_variables contains variable_max_size.""" + output_variables = self.rule.get("output_variables") + if not output_variables: + return False + + if isinstance(output_variables, list): + return "variable_max_size" in output_variables + elif isinstance(output_variables, dict): + return "variable_max_size" in output_variables.values() + + return False + + def _check_operations_for_variable_max_size(self): + """Check if operations contains variable_max_size operator.""" + operations = self.rule.get("operations") + if not operations: + return False + + if isinstance(operations, list): + return any( + isinstance(op, dict) and op.get("operator") == "variable_max_size" + for op in operations + ) + elif isinstance(operations, dict): + return operations.get("operator") == "variable_max_size" + + return False + + def _check_conditions_for_variable_max_size(self, conditions): + """ + Recursively check conditions for variable_max_size references. + Handles both ConditionComposite objects and dict/list structures. + """ + # If it's a ConditionComposite object, use its methods + if hasattr(conditions, "values"): + # Get all condition values as a flat list of dicts + condition_values = conditions.values() + for condition_dict in condition_values: + if self._contains_variable_max_size(condition_dict): + return True + # If it's a dict, check recursively + elif isinstance(conditions, dict): + if self._contains_variable_max_size(conditions): + return True + # If it's a list, check each item + elif isinstance(conditions, list): + for item in conditions: + if self._check_conditions_for_variable_max_size(item): + return True + + return False + + def _contains_variable_max_size(self, data): + """ + Check if data contains 'variable_max_size' reference. + Handles strings, dictionaries, and lists. + """ + if data == "variable_max_size": + return True + elif isinstance(data, dict): + for key, value in data.items(): + if value == "variable_max_size": + return True + # Recursively check nested structures + if isinstance(value, (dict, list)): + if self._check_conditions_for_variable_max_size(value): + return True + elif isinstance(data, list): + for item in data: + if self._contains_variable_max_size(item): + return True + + return False + def _add_variable_max_size(self, variables_metadata): """ Add variable_max_size column to the variables metadata. From 8ab7f49e1d504f4b7741841a59eb798df7d54f05 Mon Sep 17 00:00:00 2001 From: alexfurmenkov Date: Wed, 8 Apr 2026 16:07:09 +0200 Subject: [PATCH 3/4] Add unit tests for variable_max_size computation in VariablesMetadataDatasetBuilder --- ...test_variables_metadata_dataset_builder.py | 390 ++++++++++++++++++ 1 file changed, 390 insertions(+) create mode 100644 tests/unit/test_dataset_builders/test_variables_metadata_dataset_builder.py diff --git a/tests/unit/test_dataset_builders/test_variables_metadata_dataset_builder.py b/tests/unit/test_dataset_builders/test_variables_metadata_dataset_builder.py new file mode 100644 index 000000000..cb427d59d --- /dev/null +++ b/tests/unit/test_dataset_builders/test_variables_metadata_dataset_builder.py @@ -0,0 +1,390 @@ +from unittest.mock import MagicMock, patch +import pandas as pd +from cdisc_rules_engine.dataset_builders.variables_metadata_dataset_builder import ( + VariablesMetadataDatasetBuilder, +) +from cdisc_rules_engine.models.dataset.pandas_dataset import PandasDataset +from cdisc_rules_engine.services.cache.in_memory_cache_service import ( + InMemoryCacheService, +) +from cdisc_rules_engine.services.data_services import LocalDataService +from cdisc_rules_engine.models.rule_conditions import ConditionCompositeFactory + + +def create_data_service_mock(mock_get_vars, mock_get_dataset): + """Helper to create data_service mock with all required attributes.""" + svc = MagicMock(spec=LocalDataService) + svc.get_variables_metadata = mock_get_vars + svc.get_dataset = mock_get_dataset + svc.dataset_implementation = PandasDataset + return svc + + +@patch("cdisc_rules_engine.services.data_services.LocalDataService.get_dataset") +@patch( + "cdisc_rules_engine.services.data_services.LocalDataService.get_variables_metadata" +) +def test_variables_metadata_without_max_size(mock_get_vars, mock_get_ds): + """Test that variable_max_size is NOT computed when not required.""" + mock_get_vars.return_value = PandasDataset( + pd.DataFrame( + { + "variable_name": ["STUDYID", "USUBJID", "AETERM"], + "variable_label": ["Study ID", "Subject ID", "AE Term"], + "variable_size": [16, 20, 200], + "variable_order_number": [1, 2, 3], + "variable_data_type": ["Char", "Char", "Char"], + "variable_format": ["", "", ""], + } + ) + ) + + conditions_dict = {"all": [{"name": "get_dataset", "operator": "non_empty"}]} + + rule = { + "operations": None, + "conditions": ConditionCompositeFactory.get_condition_composite( + conditions_dict + ), + "output_variables": ["variable_name", "variable_size"], + } + + builder = VariablesMetadataDatasetBuilder( + rule=rule, + data_service=create_data_service_mock(mock_get_vars, mock_get_ds), + cache_service=InMemoryCacheService(), + rule_processor=MagicMock(), + data_processor=None, + dataset_path="/test/ae.xpt", + datasets=[], + dataset_metadata=MagicMock(), + define_xml_path=None, + standard="sdtmig", + standard_version="3-4", + standard_substandard=None, + ) + + result = builder.build() + + mock_get_ds.assert_not_called() + assert "variable_max_size" not in result.data.columns + assert "variable_name" in result.data.columns + assert len(result.data) == 3 + + +@patch("cdisc_rules_engine.services.data_services.LocalDataService.get_dataset") +@patch( + "cdisc_rules_engine.services.data_services.LocalDataService.get_variables_metadata" +) +def test_variables_metadata_with_max_size_in_operations(mock_get_vars, mock_get_ds): + """Test variable_max_size IS computed when in operations.""" + mock_get_vars.return_value = PandasDataset( + pd.DataFrame( + { + "variable_name": ["STUDYID", "USUBJID"], + "variable_label": ["Study ID", "Subject ID"], + "variable_size": [16, 20], + "variable_order_number": [1, 2], + "variable_data_type": ["Char", "Char"], + "variable_format": ["", ""], + } + ) + ) + + mock_get_ds.return_value = PandasDataset( + pd.DataFrame( + { + "STUDYID": ["STUDY001", "STUDY002"], + "USUBJID": ["SUBJ-001", "SUBJ-002"], + } + ) + ) + + conditions_dict = {"all": [{"name": "get_dataset", "operator": "non_empty"}]} + + rule = { + "operations": [{"operator": "variable_max_size"}], + "conditions": ConditionCompositeFactory.get_condition_composite( + conditions_dict + ), + "output_variables": ["variable_name", "variable_max_size"], + } + + builder = VariablesMetadataDatasetBuilder( + rule=rule, + data_service=create_data_service_mock(mock_get_vars, mock_get_ds), + cache_service=InMemoryCacheService(), + rule_processor=MagicMock(), + data_processor=None, + dataset_path="/test/ae.xpt", + datasets=[], + dataset_metadata=MagicMock(), + define_xml_path=None, + standard="sdtmig", + standard_version="3-4", + standard_substandard=None, + ) + + result = builder.build() + + mock_get_ds.assert_called_once() + assert "variable_max_size" in result.data.columns + assert ( + result.data[result.data["variable_name"] == "STUDYID"][ + "variable_max_size" + ].values[0] + == 8 + ) + assert ( + result.data[result.data["variable_name"] == "USUBJID"][ + "variable_max_size" + ].values[0] + == 8 + ) + + +@patch("cdisc_rules_engine.services.data_services.LocalDataService.get_dataset") +@patch( + "cdisc_rules_engine.services.data_services.LocalDataService.get_variables_metadata" +) +def test_variables_metadata_with_max_size_in_output_variables( + mock_get_vars, mock_get_ds +): + """Test variable_max_size IS computed when in output_variables.""" + mock_get_vars.return_value = PandasDataset( + pd.DataFrame( + { + "variable_name": ["STUDYID"], + "variable_label": ["Study ID"], + "variable_size": [16], + "variable_order_number": [1], + "variable_data_type": ["Char"], + "variable_format": [""], + } + ) + ) + + mock_get_ds.return_value = PandasDataset(pd.DataFrame({"STUDYID": ["STUDY001"]})) + + conditions_dict = {"all": [{"name": "get_dataset", "operator": "non_empty"}]} + + rule = { + "operations": None, + "conditions": ConditionCompositeFactory.get_condition_composite( + conditions_dict + ), + "output_variables": ["variable_max_size"], + } + + builder = VariablesMetadataDatasetBuilder( + rule=rule, + data_service=create_data_service_mock(mock_get_vars, mock_get_ds), + cache_service=InMemoryCacheService(), + rule_processor=MagicMock(), + data_processor=None, + dataset_path="/test/ae.xpt", + datasets=[], + dataset_metadata=MagicMock(), + define_xml_path=None, + standard="sdtmig", + standard_version="3-4", + standard_substandard=None, + ) + + result = builder.build() + + mock_get_ds.assert_called_once() + assert "variable_max_size" in result.data.columns + + +@patch("cdisc_rules_engine.services.data_services.LocalDataService.get_dataset") +@patch( + "cdisc_rules_engine.services.data_services.LocalDataService.get_variables_metadata" +) +def test_variables_metadata_with_max_size_in_conditions_dict( + mock_get_vars, mock_get_ds +): + """Test variable_max_size IS computed when in conditions (dict).""" + mock_get_vars.return_value = PandasDataset( + pd.DataFrame( + { + "variable_name": ["STUDYID"], + "variable_label": ["Study ID"], + "variable_size": [16], + "variable_order_number": [1], + "variable_data_type": ["Char"], + "variable_format": [""], + } + ) + ) + + mock_get_ds.return_value = PandasDataset(pd.DataFrame({"STUDYID": ["STUDY001"]})) + + conditions_dict = { + "all": [ + { + "name": "get_dataset", + "operator": "equal_to", + "value": {"target": "variable_size", "comparator": "variable_max_size"}, + } + ] + } + + rule = { + "operations": None, + "conditions": ConditionCompositeFactory.get_condition_composite( + conditions_dict + ), + "output_variables": None, + } + + builder = VariablesMetadataDatasetBuilder( + rule=rule, + data_service=create_data_service_mock(mock_get_vars, mock_get_ds), + cache_service=InMemoryCacheService(), + rule_processor=MagicMock(), + data_processor=None, + dataset_path="/test/ae.xpt", + datasets=[], + dataset_metadata=MagicMock(), + define_xml_path=None, + standard="sdtmig", + standard_version="3-4", + standard_substandard=None, + ) + + result = builder.build() + + mock_get_ds.assert_called_once() + assert "variable_max_size" in result.data.columns + + +@patch("cdisc_rules_engine.services.data_services.LocalDataService.get_dataset") +@patch( + "cdisc_rules_engine.services.data_services.LocalDataService.get_variables_metadata" +) +def test_variables_metadata_handles_nulls(mock_get_vars, mock_get_ds): + """Test variable_max_size handles null values correctly.""" + mock_get_vars.return_value = PandasDataset( + pd.DataFrame( + { + "variable_name": ["STUDYID", "AETERM"], + "variable_label": ["Study ID", "AE Term"], + "variable_size": [16, 200], + "variable_order_number": [1, 2], + "variable_data_type": ["Char", "Char"], + "variable_format": ["", ""], + } + ) + ) + + mock_get_ds.return_value = PandasDataset( + pd.DataFrame( + { + "STUDYID": ["STUDY001", None, "STUDY002"], + "AETERM": [None, None, "Hea"], + } + ) + ) + + conditions_dict = {"all": [{"name": "get_dataset", "operator": "non_empty"}]} + + rule = { + "operations": [{"operator": "variable_max_size"}], + "conditions": ConditionCompositeFactory.get_condition_composite( + conditions_dict + ), + "output_variables": ["variable_max_size"], + } + + builder = VariablesMetadataDatasetBuilder( + rule=rule, + data_service=create_data_service_mock(mock_get_vars, mock_get_ds), + cache_service=InMemoryCacheService(), + rule_processor=MagicMock(), + data_processor=None, + dataset_path="/test/ae.xpt", + datasets=[], + dataset_metadata=MagicMock(), + define_xml_path=None, + standard="sdtmig", + standard_version="3-4", + standard_substandard=None, + ) + + result = builder.build() + + assert ( + result.data[result.data["variable_name"] == "STUDYID"][ + "variable_max_size" + ].values[0] + == 8 + ) + assert ( + result.data[result.data["variable_name"] == "AETERM"][ + "variable_max_size" + ].values[0] + == 3 + ) + + +@patch("cdisc_rules_engine.services.data_services.LocalDataService.get_dataset") +@patch( + "cdisc_rules_engine.services.data_services.LocalDataService.get_variables_metadata" +) +def test_variables_metadata_handles_missing_columns(mock_get_vars, mock_get_ds): + """Test variable_max_size handles variables not in dataset.""" + mock_get_vars.return_value = PandasDataset( + pd.DataFrame( + { + "variable_name": ["STUDYID", "MISSINGVAR"], + "variable_label": ["Study ID", "Missing"], + "variable_size": [16, 20], + "variable_order_number": [1, 2], + "variable_data_type": ["Char", "Char"], + "variable_format": ["", ""], + } + ) + ) + + mock_get_ds.return_value = PandasDataset(pd.DataFrame({"STUDYID": ["STUDY001"]})) + + conditions_dict = {"all": [{"name": "get_dataset", "operator": "non_empty"}]} + + rule = { + "operations": [{"operator": "variable_max_size"}], + "conditions": ConditionCompositeFactory.get_condition_composite( + conditions_dict + ), + "output_variables": ["variable_max_size"], + } + + builder = VariablesMetadataDatasetBuilder( + rule=rule, + data_service=create_data_service_mock(mock_get_vars, mock_get_ds), + cache_service=InMemoryCacheService(), + rule_processor=MagicMock(), + data_processor=None, + dataset_path="/test/ae.xpt", + datasets=[], + dataset_metadata=MagicMock(), + define_xml_path=None, + standard="sdtmig", + standard_version="3-4", + standard_substandard=None, + ) + + result = builder.build() + + assert ( + result.data[result.data["variable_name"] == "STUDYID"][ + "variable_max_size" + ].values[0] + == 8 + ) + assert ( + result.data[result.data["variable_name"] == "MISSINGVAR"][ + "variable_max_size" + ].values[0] + == 0 + ) From e25bfac5a5d977f998485c1ef34828641a971df7 Mon Sep 17 00:00:00 2001 From: alexfurmenkov Date: Wed, 15 Apr 2026 14:35:42 +0200 Subject: [PATCH 4/4] Add variable_max_size to metadata and documentation for enhanced variable checks --- resources/schema/rule-merged/MetaVariables.json | 4 ++++ resources/schema/rule-merged/Rule_Type.json | 2 +- resources/schema/rule/MetaVariables.json | 1 + resources/schema/rule/MetaVariables.md | 4 ++++ resources/schema/rule/Rule_Type.md | 1 + 5 files changed, 11 insertions(+), 1 deletion(-) diff --git a/resources/schema/rule-merged/MetaVariables.json b/resources/schema/rule-merged/MetaVariables.json index b3cab88dc..c2cdb863c 100644 --- a/resources/schema/rule-merged/MetaVariables.json +++ b/resources/schema/rule-merged/MetaVariables.json @@ -254,6 +254,10 @@ "const": "variable_label", "markdownDescription": "\nVariable long label\n" }, + { + "const": "variable_max_size", + "markdownDescription": "\nMaximum length of actual data values in the variable\n" + }, { "const": "variable_name", "markdownDescription": "\nVariable short name\n" diff --git a/resources/schema/rule-merged/Rule_Type.json b/resources/schema/rule-merged/Rule_Type.json index 8dfb5afd5..edbe6e0de 100644 --- a/resources/schema/rule-merged/Rule_Type.json +++ b/resources/schema/rule-merged/Rule_Type.json @@ -60,7 +60,7 @@ { "const": "Variable Metadata Check", "title": "Content metadata at variable level", - "markdownDescription": "\n#### Columns\n\n- `variable_name`\n- `variable_order_number`\n- `variable_label`\n- `variable_size`\n- `variable_data_type`\n- `variable_format`\n\n#### Rule Macro\n\nChecks variable-level metadata sourced from the submission dataset contents.\n\n#### Example\n\n```yaml\n- name: variable_label\n operator: longer_than\n value: 40\n```\n" + "markdownDescription": "\n#### Columns\n\n- `variable_name`\n- `variable_order_number`\n- `variable_label`\n- `variable_size`\n- `variable_data_type`\n- `variable_format`\n- `variable_max_size` (if needed by the rule)\n\n#### Rule Macro\n\nChecks variable-level metadata sourced from the submission dataset contents.\n\n#### Example\n\n```yaml\n- name: variable_label\n operator: longer_than\n value: 40\n```\n" }, { "const": "Variable Metadata Check against Define XML", diff --git a/resources/schema/rule/MetaVariables.json b/resources/schema/rule/MetaVariables.json index cc28cfca3..1d7ea933f 100644 --- a/resources/schema/rule/MetaVariables.json +++ b/resources/schema/rule/MetaVariables.json @@ -159,6 +159,7 @@ { "const": "variable_has_empty_values" }, { "const": "variable_is_empty" }, { "const": "variable_label" }, + { "const": "variable_max_size" }, { "const": "variable_name" }, { "const": "variable_order_number" diff --git a/resources/schema/rule/MetaVariables.md b/resources/schema/rule/MetaVariables.md index 928ba3d32..8da17739a 100644 --- a/resources/schema/rule/MetaVariables.md +++ b/resources/schema/rule/MetaVariables.md @@ -258,6 +258,10 @@ True/False value indicating whether a variable is completely empty Variable long label +## variable_max_size + +Maximum length of actual data values in the variable + ## variable_name Variable short name diff --git a/resources/schema/rule/Rule_Type.md b/resources/schema/rule/Rule_Type.md index d9b838dd7..a313542c0 100644 --- a/resources/schema/rule/Rule_Type.md +++ b/resources/schema/rule/Rule_Type.md @@ -209,6 +209,7 @@ Pairs record-level data values from the submission datasets with dataset metadat - `variable_size` - `variable_data_type` - `variable_format` +- `variable_max_size` (if needed by the rule) #### Rule Macro