From 129132dd3a7eb4646d7ec4972999827e4c6e6db1 Mon Sep 17 00:00:00 2001
From: Jonathon Broughton <760691+jsdbroughton@users.noreply.github.com>
Date: Mon, 12 May 2025 15:53:33 +0100
Subject: [PATCH] Fixes (#63)
* Improves rule number handling
Adds a fallback mechanism for retrieving rule numbers.
This ensures the system can handle cases where the primary
"Rule Number" field is missing or empty, defaulting to "Rule #"
to maintain data integrity.
Also corrects some docstring formatting.
* Improves rule processing efficiency
Avoids unnecessary rule processing by checking rule severity against the minimum configured severity level. Also ensures that results are only attached to failed objects if they exist and meet the minimum severity criteria. Addresses a potential issue where rules with no "Report Severity" column could cause errors, by considering an alternative "Severity" column.
* Adds Python compatibility inspection
Ensures that the project is compatible with Python 3 by adding a compatibility inspection setting.
This will help to identify and address any potential compatibility issues early on.
* Updates integration test URL and severity.
Updates the default URL used in the integration test to a new speckle model checker endpoint.
Changes the minimum severity level from warning to info, increasing the detail of reported results.
---
.idea/misc.xml | 3 +++
src/rule_processor.py | 18 ++++++++++++------
src/spreadsheet.py | 12 ++++++------
tests/test_function.py | 6 ++++--
4 files changed, 25 insertions(+), 14 deletions(-)
diff --git a/.idea/misc.xml b/.idea/misc.xml
index 379246e..676b787 100644
--- a/.idea/misc.xml
+++ b/.idea/misc.xml
@@ -4,4 +4,7 @@
+
+
+
\ No newline at end of file
diff --git a/src/rule_processor.py b/src/rule_processor.py
index 59bf1d3..8e845bb 100644
--- a/src/rule_processor.py
+++ b/src/rule_processor.py
@@ -95,7 +95,7 @@ def evaluate_condition(
Returns:
True if the condition is met, False otherwise
"""
- property_name = condition["Property Name"]
+ property_name = condition.get("Property Name", condition.get("Property Path"))
predicate_key = condition["Predicate"]
value = condition["Value"]
@@ -263,21 +263,27 @@ def apply_rules_to_objects(
rules_processed += 1
# Ensure rule_group has necessary columns
- if "Message" not in rule_group.columns or "Report Severity" not in rule_group.columns:
+ if "Message" not in rule_group.columns or (
+ "Report Severity" not in rule_group.columns and "Severity" not in rule_group.columns
+ ):
continue # Or raise an exception if these columns are mandatory
- pass_objects, fail_objects = process_rule(speckle_objects, rule_group)
-
# Get the severity level for this rule
rule_severity = get_severity(rule_group.iloc[-1])
rule_severity_level = severity_levels[MinimumSeverity(rule_severity.value)]
+ # Check if the rule severity level meets the minimum severity level - no point in processing lower severity rules
+ if rule_severity_level < min_severity_level:
+ continue
+
+ pass_objects, fail_objects = process_rule(speckle_objects, rule_group)
+
# For passing objects, only attach if we're showing all levels (INFO)
if minimum_severity == MinimumSeverity.INFO:
attach_results(pass_objects, rule_group.iloc[-1], rule_id_str, automate_context, True)
# For failing objects, attach if they meet minimum severity threshold
- if rule_severity_level >= min_severity_level:
+ if len(fail_objects) and rule_severity_level >= min_severity_level:
attach_results(fail_objects, rule_group.iloc[-1], rule_id_str, automate_context, False)
if len(pass_objects) == 0 and len(fail_objects) == 0 and not hide_skipped:
@@ -323,7 +329,7 @@ def get_severity(rule_info: pd.Series) -> SeverityLevel:
Returns:
Appropriate SeverityLevel enum value
"""
- severity = rule_info.get("Report Severity") # Extract severity from input data
+ severity = rule_info.get("Report Severity") or rule_info.get("Severity") # Extract severity from input data
# If severity is None or not a string (e.g., numeric input), default to ERROR
if not isinstance(severity, str):
diff --git a/src/spreadsheet.py b/src/spreadsheet.py
index d4167b8..5fbdb64 100644
--- a/src/spreadsheet.py
+++ b/src/spreadsheet.py
@@ -64,8 +64,10 @@ def process_rule_numbers(df: DataFrame) -> DataFrame:
# Get slice of rows for this group
group_slice = df.iloc[start_idx:end_idx]
- # Try to get rule number from first row
- group_rule_num = group_slice["Rule Number"].iloc[0]
+ # Try to get rule number from first row, fall back to "Rule #"
+ group_rule_num = (
+ group_slice["Rule Number"].iloc[0] if not pd.isna(group_slice["Rule Number"].iloc[0]) else "Rule #"
+ )
if pd.isna(group_rule_num):
# If no rule number, generate next available number
@@ -90,8 +92,7 @@ def process_rule_numbers(df: DataFrame) -> DataFrame:
def validate_rule_numbers(df: DataFrame) -> list[str]:
- """ "
- Validate rule numbers and return any warnings or errors.
+ """Validate rule numbers and return any warnings or errors.
This checks for issues like:
1. Missing rule numbers
@@ -128,8 +129,7 @@ def validate_rule_numbers(df: DataFrame) -> list[str]:
def read_rules_from_spreadsheet(url: str) -> tuple[DataFrameGroupBy, list[str]] | tuple[None, list[str]]:
- """ "
- Reads rules from a TSV file at the provided URL, processes them, and returns grouped rules.
+ """Reads rules from a TSV file at the provided URL, processes them, and returns grouped rules.
This function is the main entry point for rule loading:
1. Reads the TSV file from the provided URL
diff --git a/tests/test_function.py b/tests/test_function.py
index c8d78e4..784d137 100644
--- a/tests/test_function.py
+++ b/tests/test_function.py
@@ -30,12 +30,14 @@ class TestFunction:
"""Run an integration test for the automate function."""
automation_context = AutomationContext.initialize(test_automation_run_data, test_automation_token)
- default_url: str = "https://drive.google.com/uc?export=download&id=1hiPSw23eOaqd27QD_YsXvZg9PWm7_XBx"
+ default_url: str = (
+ "https://speckle-model-checker-cedxvz7lzq-ew.a.run.app/r/6hdycwPELyTIT7Ueedh0UsWdJlTBefwSjDlcnd8LXGg/tsv"
+ )
automate_sdk = run_function(
automation_context,
automate_function,
- FunctionInputs(spreadsheet_url=default_url, minimum_severity=MinimumSeverity.WARNING, hide_skipped=True),
+ FunctionInputs(spreadsheet_url=default_url, minimum_severity=MinimumSeverity.INFO, hide_skipped=True),
)
assert automate_sdk.run_status == AutomationStatus.SUCCEEDED