diff --git a/cdisc_rules_engine/models/rule.py b/cdisc_rules_engine/models/rule.py index 090af8d5a..7c3c89ee6 100644 --- a/cdisc_rules_engine/models/rule.py +++ b/cdisc_rules_engine/models/rule.py @@ -35,11 +35,21 @@ def __init__(self, record_params: dict): self.output_variables: dict = record_params.get("output_variables") self.grouping_variables: List[str] = record_params.get("grouping_variables", []) + @classmethod + def _get_key(cls, obj: dict, space_key: str, default=None): + """Get a value by key, supporting both space-format ('Rule Type') and + underscore-format ('Rule_Type') for backward compatibility with the + CDISC Library API which returns underscore keys.""" + if space_key in obj: + return obj[space_key] + return obj.get(space_key.replace(" ", "_"), default) + @classmethod def from_cdisc_metadata(cls, rule_metadata: dict) -> dict: if cls.is_cdisc_rule_metadata(rule_metadata): - rule_metadata = cls.spaces_to_underscores(rule_metadata) authorities = rule_metadata.get("Authorities", []) + scope = rule_metadata.get("Scope", {}) + outcome = rule_metadata.get("Outcome", {}) executable_rule = { "core_id": rule_metadata.get("Core", {}).get("Id"), "author": "CDISC", @@ -49,49 +59,36 @@ def from_cdisc_metadata(cls, rule_metadata: dict) -> dict: "description": rule_metadata.get("Description"), "authorities": authorities, "standards": cls.parse_standards(authorities), - "classes": rule_metadata.get("Scope", {}).get("Classes"), - "domains": rule_metadata.get("Scope", {}).get("Domains"), - "entities": rule_metadata.get("Scope", {}).get("Entities"), - "rule_type": rule_metadata.get("Rule_Type"), + "classes": scope.get("Classes"), + "domains": scope.get("Domains"), + "entities": scope.get("Entities"), + "rule_type": cls._get_key(rule_metadata, "Rule Type"), "conditions": cls.parse_conditions(rule_metadata.get("Check")), - "actions": cls.parse_actions(rule_metadata.get("Outcome")), - "use_case": rule_metadata.get("Scope", {}).get("Use_Case"), - "data_structures": rule_metadata.get("Scope", {}).get( - "Data_Structures" - ), + "actions": cls.parse_actions(outcome), + "use_case": cls._get_key(scope, "Use Case"), + "data_structures": cls._get_key(scope, "Data Structures"), "status": rule_metadata.get("Core", {}).get("Status", {}), } if "Operations" in rule_metadata: executable_rule["operations"] = rule_metadata.get("Operations") - if "Match_Datasets" in rule_metadata: - executable_rule["datasets"] = cls.parse_datasets( - rule_metadata.get("Match_Datasets") - ) - if "Output_Variables" in rule_metadata.get("Outcome", {}): - executable_rule["output_variables"] = rule_metadata.get("Outcome", {})[ - "Output_Variables" - ] - if "Grouping_Variables" in rule_metadata: - executable_rule["grouping_variables"] = rule_metadata.get( - "Grouping_Variables" - ) + match_datasets = cls._get_key(rule_metadata, "Match Datasets") + if match_datasets is not None: + executable_rule["datasets"] = cls.parse_datasets(match_datasets) + + output_variables = cls._get_key(outcome, "Output Variables") + if output_variables is not None: + executable_rule["output_variables"] = output_variables + + grouping_variables = cls._get_key(rule_metadata, "Grouping Variables") + if grouping_variables is not None: + executable_rule["grouping_variables"] = grouping_variables + return executable_rule else: return rule_metadata - @classmethod - def spaces_to_underscores(cls, obj): - if isinstance(obj, dict): - return { - key.replace(" ", "_"): cls.spaces_to_underscores(value) - for key, value in obj.items() - } - if isinstance(obj, list): - return [cls.spaces_to_underscores(item) for item in obj] - return obj - @classmethod def parse_standards(cls, authorities: List[dict]) -> List[dict]: standards = [] diff --git a/cdisc_rules_engine/models/rule_validation_result.py b/cdisc_rules_engine/models/rule_validation_result.py index 01c6ca856..f48a7bb3e 100644 --- a/cdisc_rules_engine/models/rule_validation_result.py +++ b/cdisc_rules_engine/models/rule_validation_result.py @@ -45,17 +45,18 @@ def from_skipped_rule( return instance def _get_rule_ids(self, rule: Rule, org: str) -> str: - return ", ".join( - sorted( - { - reference.get("Rule_Identifier", {}).get("Id") - for authority in rule.get("authorities", []) - for standard in authority.get("Standards", []) - for reference in standard.get("References", []) - if authority.get("Organization") == org - } - ) - ) + ids = { + ( + reference.get("Rule Identifier") + or reference.get("Rule_Identifier") + or {} + ).get("Id") + for authority in rule.get("authorities", []) + for standard in authority.get("Standards", []) + for reference in standard.get("References", []) + if authority.get("Organization") == org + } + return ", ".join(sorted(id_ for id_ in ids if id_ is not None)) def to_representation(self) -> dict: return { diff --git a/scripts/script_utils.py b/scripts/script_utils.py index 99d01b2d0..f08fa5d9c 100644 --- a/scripts/script_utils.py +++ b/scripts/script_utils.py @@ -526,7 +526,7 @@ def load_and_parse_rule(rule_file): with open(rule_file, "r", encoding="utf-8") as file: if file_extension in [".yml", ".yaml"]: loaded_data = yaml.safe_load(file) - return Rule.from_cdisc_metadata(replace_yml_spaces(loaded_data)) + return Rule.from_cdisc_metadata(loaded_data) elif file_extension == ".json": return Rule.from_cdisc_metadata(json.load(file)) else: @@ -612,18 +612,6 @@ def get_max_dataset_size(dataset_paths: Iterable[str]): return max_dataset_size -def replace_yml_spaces(data): - if isinstance(data, dict): - return { - key.replace(" ", "_"): replace_yml_spaces(value) - for key, value in data.items() - } - elif isinstance(data, list): - return [replace_yml_spaces(item) for item in data] - else: - return data - - def library_metadata_not_found_message(standard, version, substandard=None): version_display = (version or "").replace("-", ".") sub_part = f" substandard {substandard}" if substandard else "" diff --git a/tests/conftest.py b/tests/conftest.py index c2f4c1d8f..b04b0ea3a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -938,14 +938,14 @@ def mock_validation_results() -> list[RuleValidationResult]: "Standards": [ { "References": [ - {"Rule_Identifier": {"Id": "CDISCRuleID4"}}, - {"Rule_Identifier": {"Id": "CDISCRuleID3"}}, + {"Rule Identifier": {"Id": "CDISCRuleID4"}}, + {"Rule Identifier": {"Id": "CDISCRuleID3"}}, ] }, { "References": [ - {"Rule_Identifier": {"Id": "CDISCRuleID2"}}, - {"Rule_Identifier": {"Id": "CDISCRuleID1"}}, + {"Rule Identifier": {"Id": "CDISCRuleID2"}}, + {"Rule Identifier": {"Id": "CDISCRuleID1"}}, ] }, ], @@ -955,8 +955,8 @@ def mock_validation_results() -> list[RuleValidationResult]: "Standards": [ { "References": [ - {"Rule_Identifier": {"Id": "FDARuleID1"}}, - {"Rule_Identifier": {"Id": "FDARuleID2"}}, + {"Rule Identifier": {"Id": "FDARuleID1"}}, + {"Rule Identifier": {"Id": "FDARuleID2"}}, ] } ], @@ -998,14 +998,14 @@ def mock_validation_results() -> list[RuleValidationResult]: "Standards": [ { "References": [ - {"Rule_Identifier": {"Id": "CDISCRuleID4"}}, - {"Rule_Identifier": {"Id": "CDISCRuleID3"}}, + {"Rule Identifier": {"Id": "CDISCRuleID4"}}, + {"Rule Identifier": {"Id": "CDISCRuleID3"}}, ] }, { "References": [ - {"Rule_Identifier": {"Id": "CDISCRuleID2"}}, - {"Rule_Identifier": {"Id": "CDISCRuleID1"}}, + {"Rule Identifier": {"Id": "CDISCRuleID2"}}, + {"Rule Identifier": {"Id": "CDISCRuleID1"}}, ] }, ], @@ -1015,8 +1015,8 @@ def mock_validation_results() -> list[RuleValidationResult]: "Standards": [ { "References": [ - {"Rule_Identifier": {"Id": "FDARuleID1"}}, - {"Rule_Identifier": {"Id": "FDARuleID2"}}, + {"Rule Identifier": {"Id": "FDARuleID1"}}, + {"Rule Identifier": {"Id": "FDARuleID2"}}, ] } ], diff --git a/tests/resources/CoreIssue295/SEND4.json b/tests/resources/CoreIssue295/SEND4.json index 51a7d99d2..a167c1c56 100644 --- a/tests/resources/CoreIssue295/SEND4.json +++ b/tests/resources/CoreIssue295/SEND4.json @@ -21,9 +21,7 @@ "Description": "Raise an error when the data type in the define.xml does not correspond to the data type in the dataset", "Outcome": { "Message": "Variable datatype in the define.xml does not correspond to the data type in the dataset", - "Output_Variables": [ - "variable_name" - ] + "Output Variables": ["variable_name"] }, "Sensitivity": "Record", "Executability": "Fully Executable", @@ -45,7 +43,7 @@ "Cited_Guidance": "The define.xml specification includes seven distinct attributes to describe variable-level metadata: ..." } ], - "Rule_Identifier": { + "Rule Identifier": { "Id": "SEND4", "Version": "1" } @@ -57,15 +55,11 @@ ], "Scope": { "Classes": { - "Include": [ - "ALL" - ] + "Include": ["ALL"] }, "Domains": { - "Include": [ - "ALL" - ] + "Include": ["ALL"] } }, - "Rule_Type": "Variable Metadata Check against Define XML" -} \ No newline at end of file + "Rule Type": "Variable Metadata Check against Define XML" +} diff --git a/tests/resources/Rule-CG0027.json b/tests/resources/Rule-CG0027.json index 5bedff2f4..9f4775985 100644 --- a/tests/resources/Rule-CG0027.json +++ b/tests/resources/Rule-CG0027.json @@ -20,7 +20,7 @@ "Description": "Trigger error when --SCAT is not null and --SCAT is equal to --CAT", "Outcome": { "Message": "--SCAT is equal to --CAT", - "Output_Variables": ["--CAT", "--SCAT"] + "Output Variables": ["--CAT", "--SCAT"] }, "Sensitivity": "Record", "Authorities": [ @@ -39,10 +39,10 @@ "Document": "Model v2.0", "Item": "--SCAT", "Section": "Interventions", - "Cited_Guidance": "A further grouping or classification of the category for the topic of the finding, event, or intervention. The category is in --CAT." + "Cited Guidance": "A further grouping or classification of the category for the topic of the finding, event, or intervention. The category is in --CAT." } ], - "Rule_Identifier": { + "Rule Identifier": { "Id": "CG0027", "Version": "1" } @@ -61,10 +61,10 @@ "Document": "Model v1.7", "Item": "--SCAT", "Section": "2.2.1", - "Cited_Guidance": "Used to define a further categorization of --CAT values." + "Cited Guidance": "Used to define a further categorization of --CAT values." } ], - "Rule_Identifier": { + "Rule Identifier": { "Id": "CG0027", "Version": "1" } @@ -83,10 +83,10 @@ "Document": "Model v1.4", "Item": "Table 2.2.1, --SCAT", "Section": "2.2.1", - "Cited_Guidance": "Used to define a further categorization of --CAT values." + "Cited Guidance": "Used to define a further categorization of --CAT values." } ], - "Rule_Identifier": { + "Rule Identifier": { "Id": "CG0027", "Version": "1" } @@ -104,5 +104,5 @@ "Include": ["ALL"] } }, - "Rule_Type": "Record Data" + "Rule Type": "Record Data" }