[problem] Update parsing of problem.yaml based on Kattis/problem-package-format#372 (#437)

mpsijm · mpsijm · commit 46533d9feffe · 2025-03-17T08:21:11.000+01:00
* [problem] Update parsing of problem.yaml based on Kattis/problem-package-format#372 Changes: - **C1**: Keywords are now a list of strings (we used to still parse them as a single string, woops) - **C2**: No change needed, this was just a "bug" in the human-readable text of the specification - **C3**: Add constraints to float/int types in `limits` and warnings when any of the values are out of range - **C5**: Do not allow lists to be empty (if a list-field is optional, it should be either `None` or a non-empty list) The discussion for **C4** was moved to Kattis/problem-package-format#378 and is pending consensus, and the proposals for **C6**, **Q1**, and **Q2** were dropped. * [problem] Fix parsing of ProblemSource, thanks to Thore's extra tests * [test] Add some more tests for license/rights_owner Note that I haven't thoroughly tested the combination of `license` and `rights_owner`. Similar to embargo_until, BAPCtools doesn't really do much with this information anyway, so the parser there is currently quite lenient, and as such I'll consider it out-of-scope for this PR. If others feel like improving and adding tests for this, feel free to do so 🙂
diff --git a/bin/problem.py b/bin/problem.py
@@ -1,3 +1,4 @@
+import datetime
 import re
 import sys
 import threading
@@ -89,8 +90,16 @@ class ProblemSources(list[ProblemSource]):
     def __init__(
         self,
         yaml_data: dict[str, Any],
-        problem_settings: "ProblemSettings",
     ):
+        def source_from_dict(source_dict: dict[str, str]) -> ProblemSource:
+            name = parse_setting(source_dict, "name", "")
+            if not name:
+                warn("problem.yaml: 'name' is required in source")
+            return ProblemSource(
+                name,
+                parse_optional_setting(source_dict, "url", str),
+            )
+
         parse_deprecated_setting(yaml_data, "source_url", "source.url")
         if "source" not in yaml_data:
             return
@@ -99,23 +108,17 @@ def __init__(
             return
         if isinstance(yaml_data["source"], dict):
             source = parse_setting(yaml_data, "source", dict[str, str]())
-            self.append(
-                ProblemSource(
-                    parse_setting(source, "name", ""),
-                    parse_optional_setting(source, "url", str),
-                )
-            )
+            self.append(source_from_dict(source))
             return
         if isinstance(yaml_data["source"], list):
             sources = parse_setting(yaml_data, "source", list[dict[str, str]]())
-            for raw_source in sources:
-                source = parse_setting(raw_source, "source", dict[str, str]())
-                self.append(
-                    ProblemSource(
-                        parse_setting(source, "name", ""),
-                        parse_optional_setting(source, "url", str),
-                    )
-                )
+            for i, source in enumerate(sources):
+                if isinstance(source, str):
+                    self.append(ProblemSource(source))
+                elif isinstance(source, dict):
+                    self.append(source_from_dict(source))
+                else:
+                    warn(f"problem.yaml key 'source[{i}]' does not have the correct type")
             return
         warn("problem.yaml key 'source' does not have the correct type")
 
@@ -134,31 +137,48 @@ def __init__(
         time_multipliers = parse_setting(yaml_data, "time_multipliers", dict[str, Any]())
 
         parse_deprecated_setting(yaml_data, "time_multiplier", "ac_to_time_limit")
-        self.ac_to_time_limit = parse_setting(time_multipliers, "ac_to_time_limit", 2.0)
+        self.ac_to_time_limit = parse_setting(time_multipliers, "ac_to_time_limit", 2.0, ">= 1")
         parse_deprecated_setting(yaml_data, "time_safety_margin", "time_limit_to_tle")
-        self.time_limit_to_tle = parse_setting(time_multipliers, "time_limit_to_tle", 1.5)
+        self.time_limit_to_tle = parse_setting(time_multipliers, "time_limit_to_tle", 1.5, ">= 1")
 
         check_unknown_keys(time_multipliers, "limits.time_multipliers")
 
-        time_limit = parse_optional_setting(yaml_data, "time_limit", float)  # in seconds
-        self.time_resolution: float = parse_setting(yaml_data, "time_resolution", 1.0)
-        self.memory: int = parse_setting(yaml_data, "memory", 2048)  # in MiB
-        self.output: int = parse_setting(yaml_data, "output", 8)  # in MiB
-        self.code: int = parse_setting(yaml_data, "code", 128)  # in KiB
-        self.compilation_time: int = parse_setting(yaml_data, "compilation_time", 60)  # in seconds
+        self.time_limit_is_default: bool = "time_limit" not in yaml_data
+        self.time_limit: float = parse_setting(yaml_data, "time_limit", 1.0, "> 0")  # in seconds
+        self.time_resolution: float = parse_setting(yaml_data, "time_resolution", 1.0, "> 0")
+        self.memory: int = parse_setting(yaml_data, "memory", 2048, "> 0")  # in MiB
+        self.output: int = parse_setting(yaml_data, "output", 8, "> 0")  # in MiB
+        self.code: int = parse_setting(yaml_data, "code", 128, "> 0")  # in KiB
+        self.compilation_time: int = parse_setting(
+            yaml_data, "compilation_time", 60, "> 0"
+        )  # in seconds
         self.compilation_memory: int = parse_setting(
-            yaml_data, "compilation_memory", 2048
+            yaml_data, "compilation_memory", 2048, "> 0"
         )  # in MiB
-        self.validation_time: int = parse_setting(yaml_data, "validation_time", 60)  # in seconds
-        self.validation_memory: int = parse_setting(yaml_data, "validation_memory", 2048)  # in MiB
-        self.validation_output: int = parse_setting(yaml_data, "validation_output", 8)  # in MiB
-        self.validation_passes: Optional[int] = parse_optional_setting(
-            yaml_data, "validation_passes", int
-        )
+        self.validation_time: int = parse_setting(
+            yaml_data, "validation_time", 60, "> 0"
+        )  # in seconds
+        self.validation_memory: int = parse_setting(
+            yaml_data, "validation_memory", 2048, "> 0"
+        )  # in MiB
+        self.validation_output: int = parse_setting(
+            yaml_data, "validation_output", 8, "> 0"
+        )  # in MiB
+        if problem_settings.multi_pass:
+            self.validation_passes: Optional[int] = parse_setting(
+                yaml_data, "validation_passes", 2, ">= 2"
+            )
+        elif "validation_passes" in yaml_data:
+            yaml_data.pop("validation_passes")
+            warn("limit: validation_passes is only used for multi-pass problems. SKIPPED.")
 
         # BAPCtools extensions:
-        self.generator_time: int = parse_setting(yaml_data, "generator_time", 60)  # in seconds
-        self.visualizer_time: int = parse_setting(yaml_data, "visualizer_time", 60)  # in seconds
+        self.generator_time: int = parse_setting(
+            yaml_data, "generator_time", 60, "> 0"
+        )  # in seconds
+        self.visualizer_time: int = parse_setting(
+            yaml_data, "visualizer_time", 60, "> 0"
+        )  # in seconds
 
         # warn for deprecated timelimit files
         if (problem.path / ".timelimit").is_file():
@@ -168,9 +188,6 @@ def __init__(
                 "domjudge-problem.ini is DEPRECATED. Use limits.time_limit if you want to set a timelimit."
             )
 
-        self.time_limit: float = time_limit or 1.0
-        self.time_limit_is_default: bool = time_limit is None
-
         check_unknown_keys(yaml_data, "limits")
 
         # Override limmits by command line arguments.
@@ -233,18 +250,23 @@ def __init__(
         self.uuid: str = parse_setting(yaml_data, "uuid", "")
         self.version: str = parse_setting(yaml_data, "version", "")
         self.credits: ProblemCredits = ProblemCredits(yaml_data, self)
-        self.source: ProblemSources = ProblemSources(yaml_data, self)
+        self.source: ProblemSources = ProblemSources(yaml_data)
         self.license: str = parse_setting(yaml_data, "license", "unknown")
-        self.rights_owner: str = parse_setting(yaml_data, "rights_owner", "")
+        self.rights_owner: Optional[str] = parse_optional_setting(yaml_data, "rights_owner", str)
         # Not implemented in BAPCtools. Should be a date, but we don't do anything with this anyway.
-        self.embargo_until: str = parse_setting(yaml_data, "embargo-until", "")
+        self.embargo_until: Optional[datetime.date] = parse_optional_setting(
+            yaml_data,
+            "embargo_until",
+            # Note that datetime.datetime is also valid, as subclass of datetime.date
+            datetime.date,
+        )
         self.limits = ProblemLimits(parse_setting(yaml_data, "limits", {}), problem, self)
 
         parse_deprecated_setting(
             yaml_data, "validator_flags", "output_validator_args' in 'testdata.yaml"
         )
 
-        self.keywords: str = parse_setting(yaml_data, "keywords", "")
+        self.keywords: list[str] = parse_optional_list_setting(yaml_data, "keywords", str)
         # Not implemented in BAPCtools. We always test all languges in langauges.yaml.
         self.languages: list[str] = parse_optional_list_setting(yaml_data, "languages", str)
 
@@ -271,13 +293,6 @@ def __init__(
             warn(f"invalid license: {self.license}")
             self.license = "unknown"
 
-        # Check that limits.validation_passes exists if and only if the problem is multi-pass
-        has_validation_passes = self.limits.validation_passes is not None
-        if self.multi_pass and not has_validation_passes:
-            self.limits.validation_passes = 2
-        if not self.multi_pass and has_validation_passes:
-            warn("limit: validation_passes is only used for multi_pass problems. SKIPPED.")
-
 
 # A problem.
 class Problem:
diff --git a/bin/util.py b/bin/util.py
@@ -813,9 +813,15 @@ def parse_optional_setting(yaml_data: dict[str, Any], key: str, t: type[T]) -> O
     return None
 
 
-def parse_setting(yaml_data: dict[str, Any], key: str, default: T) -> T:
+def parse_setting(
+    yaml_data: dict[str, Any], key: str, default: T, constraint: Optional[str] = None
+) -> T:
     value = parse_optional_setting(yaml_data, key, type(default))
-    return default if value is None else value
+    result = default if value is None else value
+    if constraint and not eval(f"{result} {constraint}"):
+        warn(f"value for '{key}' in problem.yaml should be {constraint} but is {value}. SKIPPED.")
+        return default
+    return result
 
 
 def parse_optional_list_setting(yaml_data: dict[str, Any], key: str, t: type[T]) -> list[T]:
@@ -829,6 +835,8 @@ def parse_optional_list_setting(yaml_data: dict[str, Any], key: str, t: type[T])
                     f"some values for key '{key}' in problem.yaml do not have type {t.__name__}. SKIPPED."
                 )
                 return []
+            if not value:
+                warn(f"value for '{key}' in problem.yaml should not be an empty list.")
             return value
         warn(f"incompatible value for key '{key}' in problem.yaml. SKIPPED.")
     return []
diff --git a/test/yaml/problem/invalid.yaml b/test/yaml/problem/invalid.yaml
@@ -24,6 +24,20 @@ yaml:
       mumbo: jumbo
 warn: "found unknown problem.yaml key: mumbo in `limits.time_multipliers`"
 
+---
+# UUID
+yaml:
+  problem_format_version: 2023-07-draft
+  name: Invalid UUID, too short
+  uuid: 12345678-abcd
+warn: "invalid uuid: 12345678-abcd"
+---
+yaml:
+  problem_format_version: 2023-07-draft
+  name: Invalid UUID, not hexadecimal
+  uuid: 12345678-abcd-efgh-ijkl-12345678
+warn: "invalid uuid: 12345678-abcd-efgh-ijkl-12345678"
+
 ---
 # Name
 yaml:
@@ -85,3 +99,91 @@ yaml:
   name: Incorrect type (dict)
   type: 42
 fatal: "problem.yaml: 'type' must be a string or a sequence"
+
+---
+# Limits
+yaml:
+  problem_format_version: 2023-07-draft
+  name: Negative time limit
+  limits:
+    time_limit: -1
+warn: "value for 'time_limit' in problem.yaml should be > 0 but is -1.0. SKIPPED."
+---
+yaml:
+  problem_format_version: 2023-07-draft
+  name: Time multiplier < 1
+  limits:
+    time_multipliers:
+      ac_to_time_limit: 0.9
+warn: "value for 'ac_to_time_limit' in problem.yaml should be >= 1 but is 0.9. SKIPPED."
+---
+yaml:
+  problem_format_version: 2023-07-draft
+  name: Only one pass for multi-pass
+  type: multi-pass
+  limits:
+    validation_passes: 1
+warn: "value for 'validation_passes' in problem.yaml should be >= 2 but is 1. SKIPPED."
+---
+yaml:
+  problem_format_version: 2023-07-draft
+  name: Fractional passes for multi-pass
+  type: multi-pass
+  limits:
+    validation_passes: 2.5
+warn: "incompatible value for key 'validation_passes' in problem.yaml. SKIPPED."
+---
+yaml:
+  problem_format_version: 2023-07-draft
+  name: validation_passes for non-multi-pass problem
+  limits:
+    validation_passes: 3
+warn: "limit: validation_passes is only used for multi-pass problems. SKIPPED."
+
+---
+# Empty list
+yaml:
+  problem_format_version: 2023-07-draft
+  name: pass-fail type from empty type
+  type: []
+warn: "value for 'type' in problem.yaml should not be an empty list."
+---
+yaml:
+  problem_format_version: 2023-07-draft
+  name: Empty list
+  keywords: []
+warn: "value for 'keywords' in problem.yaml should not be an empty list."
+
+---
+# Credits
+yaml:
+  problem_format_version: 2023-07-draft
+  name: Cannot specify multiple authors in credits
+  credits:
+    - name: Alice
+    - name: Audrey Authorson
+      email: bob@foo.bar
+warn: "incompatible value for key 'credits' in problem.yaml. SKIPPED."
+
+---
+# Source
+yaml:
+  problem_format_version: 2023-07-draft
+  name: Source must have a name
+  source:
+    - url: https://2024.nwerc.example/contest
+warn: "problem.yaml: 'name' is required in source"
+
+---
+# Embargo
+yaml:
+  problem_format_version: 2023-07-draft
+  name: Embargo is not a date
+  embargo_until: not a date
+warn: "incompatible value for key 'embargo_until' in problem.yaml. SKIPPED."
+#---
+#yaml:
+#  problem_format_version: 2023-07-draft
+#  name: Embargo date does not exist
+#  embargo_until: 2025-02-29
+# Note that this cannot be tested in this way, because the YAML parser already throws an error.
diff --git a/test/yaml/problem/valid.yaml b/test/yaml/problem/valid.yaml
