From f7170a0f3c53c12f0ecf87f5b7e63668dbd0d047 Mon Sep 17 00:00:00 2001 From: hoangtungdinh <11166240+hoangtungdinh@users.noreply.github.com> Date: Thu, 31 Oct 2024 15:45:29 +0100 Subject: [PATCH 1/4] Add the concept of last supported version Signed-off-by: hoangtungdinh <11166240+hoangtungdinh@users.noreply.github.com> --- README.md | 13 +- ...tions_connection_one_connection_element.py | 1 + qc_opendrive/main.py | 24 +- ...n_one_connection_element_v1_8_0_valid.xodr | 410 ++++++++++++++++++ tests/test_semantic_checks.py | 26 ++ 5 files changed, 467 insertions(+), 7 deletions(-) create mode 100755 tests/data/junctions_connection_one_connection_element/junctions_connection_one_connection_element_v1_8_0_valid.xodr diff --git a/README.md b/README.md index be80306..aab6f7c 100644 --- a/README.md +++ b/README.md @@ -279,12 +279,13 @@ Contributions of valid and invalid OpenDrive sample files are also welcome. New 1. Create a new Python module for each checker. 2. Specify the following global variables for the Python module -| Variable | Meaning | -| --- | --- | -| `CHECKER_ID` | The ID of the checker | -| `CHECKER_DESCRIPTION` | The description of the checker | -| `CHECKER_PRECONDITIONS` | A set of other checkers in which if any of them raise an issue, the current checker will be skipped | -| `RULE_UID` | The rule UID of the rule that the checker will check | +| Variable | Presence | Meaning | +| --- | --- | --- | +| `CHECKER_ID` | Required | The ID of the checker | +| `CHECKER_DESCRIPTION` | Required | The description of the checker | +| `CHECKER_PRECONDITIONS` | Required | A set of other checkers in which if any of them raise an issue, the current checker will be skipped | +| `RULE_UID` | Required | The rule UID of the rule that the checker will check | +| `LAST_SUPPORTED_VERSION` | Optional | The last supported version of the standard. If the input file has a version higher than the last supported version, the checker will be skipped. Do not define this variable if the checker does not have a last supported version. | 3. Implement the checker logic in the following function: diff --git a/qc_opendrive/checks/semantic/junctions_connection_one_connection_element.py b/qc_opendrive/checks/semantic/junctions_connection_one_connection_element.py index ead92e3..a738e36 100644 --- a/qc_opendrive/checks/semantic/junctions_connection_one_connection_element.py +++ b/qc_opendrive/checks/semantic/junctions_connection_one_connection_element.py @@ -19,6 +19,7 @@ CHECKER_DESCRIPTION = "Each connecting road shall be represented by exactly one element. A connecting road may contain as many lanes as required." CHECKER_PRECONDITIONS = basic_preconditions.CHECKER_PRECONDITIONS RULE_UID = "asam.net:xodr:1.7.0:junctions.connection.one_connection_element" +LAST_SUPPORTED_VERSION = "1.7.0" def _check_junctions_connection_one_connection_element( diff --git a/qc_opendrive/main.py b/qc_opendrive/main.py index 65c1091..b6cd0a7 100644 --- a/qc_opendrive/main.py +++ b/qc_opendrive/main.py @@ -73,7 +73,7 @@ def execute_checker( return - # Checker definition setting. If not satisfied then set status as SKIPPED and return + # Check definition setting. If not satisfied then set status as SKIPPED and return if required_definition_setting: schema_version = checker_data.schema_version @@ -100,6 +100,28 @@ def execute_checker( return + # Check last supported version. If not satisfied then set status as SKIPPED and return + if hasattr(checker, "LAST_SUPPORTED_VERSION"): + schema_version = checker_data.schema_version + if ( + schema_version is None + or utils.compare_versions(schema_version, checker.LAST_SUPPORTED_VERSION) + > 0 + ): + checker_data.result.set_checker_status( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=checker.CHECKER_ID, + status=StatusType.SKIPPED, + ) + + checker_data.result.add_checker_summary( + constants.BUNDLE_NAME, + checker.CHECKER_ID, + f"Version {schema_version} is higher than the last supported version {checker.LAST_SUPPORTED_VERSION}. Skip the check.", + ) + + return + # Execute checker try: checker.check_rule(checker_data) diff --git a/tests/data/junctions_connection_one_connection_element/junctions_connection_one_connection_element_v1_8_0_valid.xodr b/tests/data/junctions_connection_one_connection_element/junctions_connection_one_connection_element_v1_8_0_valid.xodr new file mode 100755 index 0000000..390cb02 --- /dev/null +++ b/tests/data/junctions_connection_one_connection_element/junctions_connection_one_connection_element_v1_8_0_valid.xodr @@ -0,0 +1,410 @@ + + +
+ + + +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ + + + + + + +
+ + + + + + + + + + + + + +
+
+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ + + + + + + +
+ + + + + + + + + + + + + +
+
+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ + + + + + + +
+ + + + + + + + + + + + + +
+
+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ + + + + + + +
+ + + + + + + + + + + + + +
+
+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ + + +
+ + + + + + + + + + + + +
+
+ + + +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ + + +
+ + + + + + + + + + + + +
+
+ + + +
+ + + + + + + + + + + + + + +
diff --git a/tests/test_semantic_checks.py b/tests/test_semantic_checks.py index 5cd6328..7ffeafa 100644 --- a/tests/test_semantic_checks.py +++ b/tests/test_semantic_checks.py @@ -492,6 +492,32 @@ def test_junctions_connection_one_connection_element( cleanup_files() +@pytest.mark.parametrize( + "target_file,issue_count,issue_xpath", + [ + ("v1_8_0_valid", 0, []), + ], +) +def test_junctions_connection_one_connection_element_last_supported_version( + target_file: str, + issue_count: int, + issue_xpath: List[str], + monkeypatch, +) -> None: + base_path = "tests/data/junctions_connection_one_connection_element/" + target_file_name = f"junctions_connection_one_connection_element_{target_file}.xodr" + rule_uid = "asam.net:xodr:1.7.0:junctions.connection.one_connection_element" + + target_file_path = os.path.join(base_path, target_file_name) + create_test_config(target_file_path) + launch_main(monkeypatch) + check_skipped( + rule_uid, + semantic.junctions_connection_one_connection_element.CHECKER_ID, + ) + cleanup_files() + + @pytest.mark.parametrize( "target_file,issue_count,issue_xpath", [ From 67d5a95e028f336308e42cf08c8f38bf87c5bf95 Mon Sep 17 00:00:00 2001 From: hoangtungdinh <11166240+hoangtungdinh@users.noreply.github.com> Date: Tue, 5 Nov 2024 15:29:50 +0100 Subject: [PATCH 2/4] Support applicable version range Signed-off-by: hoangtungdinh <11166240+hoangtungdinh@users.noreply.github.com> --- README.md | 42 +- poetry.lock | 75 ++-- pyproject.toml | 1 + qc_opendrive/__init__.py | 1 + qc_opendrive/base/utils.py | 32 -- ...tions_connection_one_connection_element.py | 2 +- qc_opendrive/main.py | 189 +++++--- qc_opendrive/version.py | 77 ++++ ...one_connection_element_v1_6_0_skipped.xodr | 410 ++++++++++++++++++ tests/test_semantic_checks.py | 3 +- tests/test_version.py | 66 +++ 11 files changed, 764 insertions(+), 134 deletions(-) create mode 100644 qc_opendrive/version.py create mode 100755 tests/data/junctions_connection_one_connection_element/junctions_connection_one_connection_element_v1_6_0_skipped.xodr create mode 100644 tests/test_version.py diff --git a/README.md b/README.md index aab6f7c..6e4d76c 100644 --- a/README.md +++ b/README.md @@ -285,9 +285,9 @@ Contributions of valid and invalid OpenDrive sample files are also welcome. New | `CHECKER_DESCRIPTION` | Required | The description of the checker | | `CHECKER_PRECONDITIONS` | Required | A set of other checkers in which if any of them raise an issue, the current checker will be skipped | | `RULE_UID` | Required | The rule UID of the rule that the checker will check | -| `LAST_SUPPORTED_VERSION` | Optional | The last supported version of the standard. If the input file has a version higher than the last supported version, the checker will be skipped. Do not define this variable if the checker does not have a last supported version. | +| `APPLICABLE_VERSION` | Optional | An optional variable to define extra constraints on the applicable version. See details below. | -3. Implement the checker logic in the following function: +1. Implement the checker logic in the following function: ```python def check_rule(checker_data: models.CheckerData) -> None: @@ -305,3 +305,41 @@ def run_checks(config: Configuration, result: Result) -> None: ``` All the checkers in this checker bundle are implemented in this way. Take a look at some of them before implementing your first checker. + +**A note on `APPLICABLE_VERSION`** + +The `APPLICABLE_VERSION` variable can be used to define additional constraints on the versions of the input files that a rule supports, in addition to the **definition setting** in the rule UID. It can be specified in the same way as the [Python Version Specifiers](https://packaging.python.org/en/latest/specifications/version-specifiers/#id5). For example: + +```python +APPLICABLE_VERSION = "<1.8.0" +APPLICABLE_VERSION = ">=1.6.0" +APPLICABLE_VERSION = "<1.8.0,>=1.6.0" +``` + +The specification consists of a series of version clauses, separated by commas. The comma is equivalent to a logical "AND" operator: a candidate version must match all given version clauses in order to match the **applicable version** as a whole. + +The **applicable version** only supports versions of the form `major.minor.patch`. For example, `1.7.0rc1` is not supported, but `1.7.0` is supported. + +The **applicable version** only supports the following comparison operators. + +* `<` smaller than **(upper bound)** +* `<=` smaller or equal than **(upper bound)** +* `>` greater than **(lower bound)** +* `>=` greater or equal than **(lower bound)** + +The **definition setting** in rule UID and the **applicable version** together define the versions of the input file in which a rule can be applied. For example, let's consider a rule UID for ASAM OpenDRIVE `asam.net:xodr:1.6.0:*`. The **definition setting** in this case is `1.6.0`. + +1. If no **applicable version** is specified, a rule will be applied starting from the **definition setting** version, up to the most recent one. + * For the example, if the `APPLICABLE_VERSION` variable does not exist, then the rule is applied to OpenDRIVE versions 1.6.0, 1.6.1, 1.7.0, 1.8.0: the internal representation of the version specifier is `>=1.6.0`. + +2. If the **applicable version** is specified, and defines only **upper bounds**, then the **definition setting** defines the lower bound + * For the example, if `APPLICABLE_VERSION = "<1.8.0"`, then the rule is applied to OpenDRIVE versions 1.6.0, 1.6.1, 1.7.0: the internal representation of the version specifier is `>=1.6.0,<1.8.0`. + +3. If the **applicable version** is specified, and defines at least one **lower bound**, then the **definition setting** is ignored. Only the **lower bounds** defined in the **applicable version** are taken into account. + * For the example, if `APPLICABLE_VERSION = ">=1.5.0"`, then the rule is applied to OpenDRIVE versions 1.5.0, 1.6.0, 1.6.1, 1.7.0, 1.8.0: the internal representation of the version specifier is `>=1.5.0`. + +| Case Number | Example Rule | `APPLICABLE_VERSION` | Internal Representation | File versions to be checked | +|-------------|-------------------------|----------------------|----------------------------------|-----------------------------------| +| 1 | `asam.net:xodr:1.6.0:*` | `""` | `">=1.6.0"` | 1.6.0, 1.6.1, 1.7.0, 1.8.0 | +| 2 | `asam.net:xodr:1.6.0:*` | `"<1.8.0"` | `">=1.6.0,<1.8.0"` | 1.6.0, 1.6.1, 1.7.0 | +| 3 | `asam.net:xodr:1.6.0:*` | `">=1.5.0"` | `">=1.5.0"` | 1.5.0, 1.6.0, 1.6.1, 1.7.0, 1.8.0 | diff --git a/poetry.lock b/poetry.lock index 3a64a5c..b453641 100644 --- a/poetry.lock +++ b/poetry.lock @@ -29,33 +29,33 @@ pydantic-xml = ">=2.11.0,<3.0.0" [[package]] name = "black" -version = "24.8.0" +version = "24.10.0" description = "The uncompromising code formatter." optional = false -python-versions = ">=3.8" +python-versions = ">=3.9" files = [ - {file = "black-24.8.0-cp310-cp310-macosx_10_9_x86_64.whl", hash = "sha256:09cdeb74d494ec023ded657f7092ba518e8cf78fa8386155e4a03fdcc44679e6"}, - {file = "black-24.8.0-cp310-cp310-macosx_11_0_arm64.whl", hash = "sha256:81c6742da39f33b08e791da38410f32e27d632260e599df7245cccee2064afeb"}, - {file = "black-24.8.0-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:707a1ca89221bc8a1a64fb5e15ef39cd755633daa672a9db7498d1c19de66a42"}, - {file = "black-24.8.0-cp310-cp310-win_amd64.whl", hash = "sha256:d6417535d99c37cee4091a2f24eb2b6d5ec42b144d50f1f2e436d9fe1916fe1a"}, - {file = "black-24.8.0-cp311-cp311-macosx_10_9_x86_64.whl", hash = "sha256:fb6e2c0b86bbd43dee042e48059c9ad7830abd5c94b0bc518c0eeec57c3eddc1"}, - {file = "black-24.8.0-cp311-cp311-macosx_11_0_arm64.whl", hash = "sha256:837fd281f1908d0076844bc2b801ad2d369c78c45cf800cad7b61686051041af"}, - {file = "black-24.8.0-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:62e8730977f0b77998029da7971fa896ceefa2c4c4933fcd593fa599ecbf97a4"}, - {file = "black-24.8.0-cp311-cp311-win_amd64.whl", hash = "sha256:72901b4913cbac8972ad911dc4098d5753704d1f3c56e44ae8dce99eecb0e3af"}, - {file = "black-24.8.0-cp312-cp312-macosx_10_9_x86_64.whl", hash = "sha256:7c046c1d1eeb7aea9335da62472481d3bbf3fd986e093cffd35f4385c94ae368"}, - {file = "black-24.8.0-cp312-cp312-macosx_11_0_arm64.whl", hash = "sha256:649f6d84ccbae73ab767e206772cc2d7a393a001070a4c814a546afd0d423aed"}, - {file = "black-24.8.0-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:2b59b250fdba5f9a9cd9d0ece6e6d993d91ce877d121d161e4698af3eb9c1018"}, - {file = "black-24.8.0-cp312-cp312-win_amd64.whl", hash = "sha256:6e55d30d44bed36593c3163b9bc63bf58b3b30e4611e4d88a0c3c239930ed5b2"}, - {file = "black-24.8.0-cp38-cp38-macosx_10_9_x86_64.whl", hash = "sha256:505289f17ceda596658ae81b61ebbe2d9b25aa78067035184ed0a9d855d18afd"}, - {file = "black-24.8.0-cp38-cp38-macosx_11_0_arm64.whl", hash = "sha256:b19c9ad992c7883ad84c9b22aaa73562a16b819c1d8db7a1a1a49fb7ec13c7d2"}, - {file = "black-24.8.0-cp38-cp38-manylinux_2_17_x86_64.manylinux2014_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:1f13f7f386f86f8121d76599114bb8c17b69d962137fc70efe56137727c7047e"}, - {file = "black-24.8.0-cp38-cp38-win_amd64.whl", hash = "sha256:f490dbd59680d809ca31efdae20e634f3fae27fba3ce0ba3208333b713bc3920"}, - {file = "black-24.8.0-cp39-cp39-macosx_10_9_x86_64.whl", hash = "sha256:eab4dd44ce80dea27dc69db40dab62d4ca96112f87996bca68cd75639aeb2e4c"}, - {file = "black-24.8.0-cp39-cp39-macosx_11_0_arm64.whl", hash = "sha256:3c4285573d4897a7610054af5a890bde7c65cb466040c5f0c8b732812d7f0e5e"}, - {file = "black-24.8.0-cp39-cp39-manylinux_2_17_x86_64.manylinux2014_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:9e84e33b37be070ba135176c123ae52a51f82306def9f7d063ee302ecab2cf47"}, - {file = "black-24.8.0-cp39-cp39-win_amd64.whl", hash = "sha256:73bbf84ed136e45d451a260c6b73ed674652f90a2b3211d6a35e78054563a9bb"}, - {file = "black-24.8.0-py3-none-any.whl", hash = "sha256:972085c618ee94f402da1af548a4f218c754ea7e5dc70acb168bfaca4c2542ed"}, - {file = "black-24.8.0.tar.gz", hash = "sha256:2500945420b6784c38b9ee885af039f5e7471ef284ab03fa35ecdde4688cd83f"}, + {file = "black-24.10.0-cp310-cp310-macosx_10_9_x86_64.whl", hash = "sha256:e6668650ea4b685440857138e5fe40cde4d652633b1bdffc62933d0db4ed9812"}, + {file = "black-24.10.0-cp310-cp310-macosx_11_0_arm64.whl", hash = "sha256:1c536fcf674217e87b8cc3657b81809d3c085d7bf3ef262ead700da345bfa6ea"}, + {file = "black-24.10.0-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:649fff99a20bd06c6f727d2a27f401331dc0cc861fb69cde910fe95b01b5928f"}, + {file = "black-24.10.0-cp310-cp310-win_amd64.whl", hash = "sha256:fe4d6476887de70546212c99ac9bd803d90b42fc4767f058a0baa895013fbb3e"}, + {file = "black-24.10.0-cp311-cp311-macosx_10_9_x86_64.whl", hash = "sha256:5a2221696a8224e335c28816a9d331a6c2ae15a2ee34ec857dcf3e45dbfa99ad"}, + {file = "black-24.10.0-cp311-cp311-macosx_11_0_arm64.whl", hash = "sha256:f9da3333530dbcecc1be13e69c250ed8dfa67f43c4005fb537bb426e19200d50"}, + {file = "black-24.10.0-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:4007b1393d902b48b36958a216c20c4482f601569d19ed1df294a496eb366392"}, + {file = "black-24.10.0-cp311-cp311-win_amd64.whl", hash = "sha256:394d4ddc64782e51153eadcaaca95144ac4c35e27ef9b0a42e121ae7e57a9175"}, + {file = "black-24.10.0-cp312-cp312-macosx_10_13_x86_64.whl", hash = "sha256:b5e39e0fae001df40f95bd8cc36b9165c5e2ea88900167bddf258bacef9bbdc3"}, + {file = "black-24.10.0-cp312-cp312-macosx_11_0_arm64.whl", hash = "sha256:d37d422772111794b26757c5b55a3eade028aa3fde43121ab7b673d050949d65"}, + {file = "black-24.10.0-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:14b3502784f09ce2443830e3133dacf2c0110d45191ed470ecb04d0f5f6fcb0f"}, + {file = "black-24.10.0-cp312-cp312-win_amd64.whl", hash = "sha256:30d2c30dc5139211dda799758559d1b049f7f14c580c409d6ad925b74a4208a8"}, + {file = "black-24.10.0-cp313-cp313-macosx_10_13_x86_64.whl", hash = "sha256:1cbacacb19e922a1d75ef2b6ccaefcd6e93a2c05ede32f06a21386a04cedb981"}, + {file = "black-24.10.0-cp313-cp313-macosx_11_0_arm64.whl", hash = "sha256:1f93102e0c5bb3907451063e08b9876dbeac810e7da5a8bfb7aeb5a9ef89066b"}, + {file = "black-24.10.0-cp313-cp313-manylinux_2_17_x86_64.manylinux2014_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:ddacb691cdcdf77b96f549cf9591701d8db36b2f19519373d60d31746068dbf2"}, + {file = "black-24.10.0-cp313-cp313-win_amd64.whl", hash = "sha256:680359d932801c76d2e9c9068d05c6b107f2584b2a5b88831c83962eb9984c1b"}, + {file = "black-24.10.0-cp39-cp39-macosx_10_9_x86_64.whl", hash = "sha256:17374989640fbca88b6a448129cd1745c5eb8d9547b464f281b251dd00155ccd"}, + {file = "black-24.10.0-cp39-cp39-macosx_11_0_arm64.whl", hash = "sha256:63f626344343083322233f175aaf372d326de8436f5928c042639a4afbbf1d3f"}, + {file = "black-24.10.0-cp39-cp39-manylinux_2_17_x86_64.manylinux2014_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:ccfa1d0cb6200857f1923b602f978386a3a2758a65b52e0950299ea014be6800"}, + {file = "black-24.10.0-cp39-cp39-win_amd64.whl", hash = "sha256:2cd9c95431d94adc56600710f8813ee27eea544dd118d45896bb734e9d7a0dc7"}, + {file = "black-24.10.0-py3-none-any.whl", hash = "sha256:3bb2b7a1f7b685f85b11fed1ef10f8a9148bceb49853e47a294a3dd963c1dd7d"}, + {file = "black-24.10.0.tar.gz", hash = "sha256:846ea64c97afe3bc677b761787993be4991810ecc7a4a937816dd6bddedc4875"}, ] [package.dependencies] @@ -69,7 +69,7 @@ typing-extensions = {version = ">=4.0.1", markers = "python_version < \"3.11\""} [package.extras] colorama = ["colorama (>=0.4.3)"] -d = ["aiohttp (>=3.7.4)", "aiohttp (>=3.7.4,!=3.9.0)"] +d = ["aiohttp (>=3.10)"] jupyter = ["ipython (>=7.8.0)", "tokenize-rt (>=3.2.0)"] uvloop = ["uvloop (>=0.15.2)"] @@ -100,13 +100,13 @@ files = [ [[package]] name = "elementpath" -version = "4.5.0" +version = "4.6.0" description = "XPath 1.0/2.0/3.0/3.1 parsers and selectors for ElementTree and lxml" optional = false python-versions = ">=3.8" files = [ - {file = "elementpath-4.5.0-py3-none-any.whl", hash = "sha256:a16438bcc6b2b3069dde204c1e105322378a108b28faea3055d1b294443babea"}, - {file = "elementpath-4.5.0.tar.gz", hash = "sha256:affdc8de95af1a4c10d1d2ed79c6fa56b59c26c7fce64b73497569e9dea46998"}, + {file = "elementpath-4.6.0-py3-none-any.whl", hash = "sha256:e578677f19ccc6ff374c4477c687c547ecbaf7b478d98abb951b7b4b45260a17"}, + {file = "elementpath-4.6.0.tar.gz", hash = "sha256:ba46bf07f66774927727ade55022b6c435fac06b2523cb3cd7689a1884d33468"}, ] [package.extras] @@ -696,6 +696,17 @@ dev = ["cython-lint (>=0.12.2)", "doit (>=0.36.0)", "mypy (==1.10.0)", "pycodest doc = ["jupyterlite-pyodide-kernel", "jupyterlite-sphinx (>=0.13.1)", "jupytext", "matplotlib (>=3.5)", "myst-nb", "numpydoc", "pooch", "pydata-sphinx-theme (>=0.15.2)", "sphinx (>=5.0.0,<=7.3.7)", "sphinx-design (>=0.4.0)"] test = ["Cython", "array-api-strict (>=2.0)", "asv", "gmpy2", "hypothesis (>=6.30)", "meson", "mpmath", "ninja", "pooch", "pytest", "pytest-cov", "pytest-timeout", "pytest-xdist", "scikit-umfpack", "threadpoolctl"] +[[package]] +name = "semver" +version = "3.0.2" +description = "Python helper for Semantic Versioning (https://semver.org)" +optional = false +python-versions = ">=3.7" +files = [ + {file = "semver-3.0.2-py3-none-any.whl", hash = "sha256:b1ea4686fe70b981f85359eda33199d60c53964284e0cfb4977d243e37cf4bf4"}, + {file = "semver-3.0.2.tar.gz", hash = "sha256:6253adb39c70f6e51afed2fa7152bcd414c411286088fb4b9effb133885ab4cc"}, +] + [[package]] name = "tomli" version = "2.0.2" @@ -734,13 +745,13 @@ files = [ [[package]] name = "xmlschema" -version = "3.4.2" +version = "3.4.3" description = "An XML Schema validator and decoder" optional = false python-versions = ">=3.8" files = [ - {file = "xmlschema-3.4.2-py3-none-any.whl", hash = "sha256:c6b4de5f8aadeb45e74229f09a2129342b446456efc5e5a27388050afdfedec8"}, - {file = "xmlschema-3.4.2.tar.gz", hash = "sha256:d35023ea504ea46127302d1297b046d023b96fec5fe4b4b690534ea85b5e9bf8"}, + {file = "xmlschema-3.4.3-py3-none-any.whl", hash = "sha256:eea4e5a1aac041b546ebe7b2eb68eb5eaebf5c5258e573cfc182375676b2e4e3"}, + {file = "xmlschema-3.4.3.tar.gz", hash = "sha256:0c638dac81c7d6c9da9a8d7544402c48cffe7ee0e13cc47fc0c18794d1395dfb"}, ] [package.dependencies] @@ -754,4 +765,4 @@ docs = ["Sphinx", "elementpath (>=4.4.0,<5.0.0)", "jinja2", "sphinx-rtd-theme"] [metadata] lock-version = "2.0" python-versions = "^3.10" -content-hash = "da96639686a715680bfa09499b7751aebce60067618f09ae60405e034ec0bc77" +content-hash = "38843d6ec103d3e0f5ff932bb4b9f2ba826d5fdd2cb7aa9858854120aa3d1d7c" diff --git a/pyproject.toml b/pyproject.toml index c047160..5d7c26f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -19,6 +19,7 @@ scipy = "^1.14.0" pyclothoids = "^0.1.5" transforms3d = "^0.4.2" xmlschema = "^3.3.1" +semver = "^3.0.0" [tool.poetry.group.dev.dependencies] pytest = "^8.2.2" diff --git a/qc_opendrive/__init__.py b/qc_opendrive/__init__.py index d071e3b..92f76df 100644 --- a/qc_opendrive/__init__.py +++ b/qc_opendrive/__init__.py @@ -7,3 +7,4 @@ from . import constants as constants from . import checks as checks from . import basic_preconditions as basic_preconditions +from . import version as version diff --git a/qc_opendrive/base/utils.py b/qc_opendrive/base/utils.py index e4f63c3..7d01594 100644 --- a/qc_opendrive/base/utils.py +++ b/qc_opendrive/base/utils.py @@ -1757,35 +1757,3 @@ def get_middle_point_xyz_at_height_zero_from_lane_by_s( def get_s_offset_from_access(access: etree._ElementTree) -> Optional[float]: return to_float(access.get("sOffset")) - - -def compare_versions(version1: str, version2: str) -> int: - """Compare two version strings like "X.x.x" - This function is to avoid comparing version string basing on lexicographical order - that could cause problem. E.g. - 1.10.0 > 1.2.0 but lexicographical comparison of string would return the opposite - - Args: - version1 (str): First string to compare - version2 (str): Second string to compare - - Returns: - int: 1 if version1 is bigger than version2. 0 if the version are the same. -1 otherwise - """ - v1_components = list(map(int, version1.split("."))) - v2_components = list(map(int, version2.split("."))) - - # Compare each component until one is greater or they are equal - for v1, v2 in zip(v1_components, v2_components): - if v1 < v2: - return -1 - elif v1 > v2: - return 1 - - # If all components are equal, compare based on length - if len(v1_components) < len(v2_components): - return -1 - elif len(v1_components) > len(v2_components): - return 1 - else: - return 0 diff --git a/qc_opendrive/checks/semantic/junctions_connection_one_connection_element.py b/qc_opendrive/checks/semantic/junctions_connection_one_connection_element.py index a738e36..daeb694 100644 --- a/qc_opendrive/checks/semantic/junctions_connection_one_connection_element.py +++ b/qc_opendrive/checks/semantic/junctions_connection_one_connection_element.py @@ -19,7 +19,7 @@ CHECKER_DESCRIPTION = "Each connecting road shall be represented by exactly one element. A connecting road may contain as many lanes as required." CHECKER_PRECONDITIONS = basic_preconditions.CHECKER_PRECONDITIONS RULE_UID = "asam.net:xodr:1.7.0:junctions.connection.one_connection_element" -LAST_SUPPORTED_VERSION = "1.7.0" +APPLICABLE_VERSION = "<=1.7.0" def _check_junctions_connection_one_connection_element( diff --git a/qc_opendrive/main.py b/qc_opendrive/main.py index b6cd0a7..6e3a7eb 100644 --- a/qc_opendrive/main.py +++ b/qc_opendrive/main.py @@ -9,9 +9,9 @@ import types from qc_baselib import Configuration, Result, StatusType -from qc_baselib.models.common import ParamType from qc_opendrive import constants +from qc_opendrive import version from qc_opendrive.checks import semantic from qc_opendrive.checks import geometry from qc_opendrive.checks import performance @@ -36,29 +36,17 @@ def args_entrypoint() -> argparse.Namespace: return parser.parse_args() -def execute_checker( - checker: types.ModuleType, - checker_data: models.CheckerData, - required_definition_setting: bool = True, -) -> None: - # Register checker - checker_data.result.register_checker( - checker_bundle_name=constants.BUNDLE_NAME, - checker_id=checker.CHECKER_ID, - description=checker.CHECKER_DESCRIPTION, - ) - - # Register rule uid - checker_data.result.register_rule_by_uid( - checker_bundle_name=constants.BUNDLE_NAME, - checker_id=checker.CHECKER_ID, - rule_uid=checker.RULE_UID, - ) - - # Check preconditions. If not satisfied then set status as SKIPPED and return - if not checker_data.result.all_checkers_completed_without_issue( +def check_preconditions( + checker: types.ModuleType, checker_data: models.CheckerData +) -> bool: + """ + Check preconditions. If not satisfied then set status as SKIPPED and return False + """ + if checker_data.result.all_checkers_completed_without_issue( checker.CHECKER_PRECONDITIONS ): + return True + else: checker_data.result.set_checker_status( checker_bundle_name=constants.BUNDLE_NAME, checker_id=checker.CHECKER_ID, @@ -71,43 +59,87 @@ def execute_checker( "Preconditions are not satisfied. Skip the check.", ) - return + return False - # Check definition setting. If not satisfied then set status as SKIPPED and return - if required_definition_setting: - schema_version = checker_data.schema_version - splitted_rule_uid = checker.RULE_UID.split(":") - if len(splitted_rule_uid) != 4: - raise RuntimeError(f"Invalid rule uid: {checker.RULE_UID}") +def check_version(checker: types.ModuleType, checker_data: models.CheckerData) -> bool: + """ + Check definition setting and applicable version. + If not satisfied then set status as SKIPPED or ERROR and return False + """ + schema_version = checker_data.schema_version - definition_setting = splitted_rule_uid[2] - if ( - schema_version is None - or utils.compare_versions(schema_version, definition_setting) < 0 - ): - checker_data.result.set_checker_status( - checker_bundle_name=constants.BUNDLE_NAME, - checker_id=checker.CHECKER_ID, - status=StatusType.SKIPPED, - ) + splitted_rule_uid = checker.RULE_UID.split(":") + if len(splitted_rule_uid) != 4: + raise RuntimeError(f"Invalid rule uid: {checker.RULE_UID}") - checker_data.result.add_checker_summary( - constants.BUNDLE_NAME, - checker.CHECKER_ID, - f"Version {schema_version} is lower than definition setting {definition_setting}. Skip the check.", - ) + definition_setting = splitted_rule_uid[2] + definition_setting_expr = f">={definition_setting}" + match_definition_setting = version.match(schema_version, definition_setting_expr) - return + applicable_version = ( + checker.APPLICABLE_VERSION if hasattr(checker, "APPLICABLE_VERSION") else None + ) - # Check last supported version. If not satisfied then set status as SKIPPED and return - if hasattr(checker, "LAST_SUPPORTED_VERSION"): - schema_version = checker_data.schema_version - if ( - schema_version is None - or utils.compare_versions(schema_version, checker.LAST_SUPPORTED_VERSION) - > 0 - ): + # Check whether applicable version specification is valid + if applicable_version is not None and not version.is_valid_version_expression( + applicable_version + ): + checker_data.result.set_checker_status( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=checker.CHECKER_ID, + status=StatusType.ERROR, + ) + + checker_data.result.add_checker_summary( + constants.BUNDLE_NAME, + checker.CHECKER_ID, + f"The applicable version {applicable_version} is not valid. Skip the check.", + ) + + return False + + # Check whether definition setting specification is valid + if not version.is_valid_version_expression(definition_setting_expr): + checker_data.result.set_checker_status( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=checker.CHECKER_ID, + status=StatusType.ERROR, + ) + + checker_data.result.add_checker_summary( + constants.BUNDLE_NAME, + checker.CHECKER_ID, + f"The definition setting {definition_setting} is not valid. Skip the check.", + ) + + return False + + match_applicable_version = ( + version.match(schema_version, applicable_version) + if applicable_version is not None + else True + ) + + # First, check applicable version + if not match_applicable_version: + checker_data.result.set_checker_status( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=checker.CHECKER_ID, + status=StatusType.SKIPPED, + ) + + checker_data.result.add_checker_summary( + constants.BUNDLE_NAME, + checker.CHECKER_ID, + f"Version {schema_version} is not valid according to the applicable version {applicable_version}. Skip the check.", + ) + + return False + + # Check definition setting if there is no applicable version or applicable version has no lower bound + if applicable_version is None or not version.has_lower_bound(applicable_version): + if not match_definition_setting: checker_data.result.set_checker_status( checker_bundle_name=constants.BUNDLE_NAME, checker_id=checker.CHECKER_ID, @@ -117,9 +149,42 @@ def execute_checker( checker_data.result.add_checker_summary( constants.BUNDLE_NAME, checker.CHECKER_ID, - f"Version {schema_version} is higher than the last supported version {checker.LAST_SUPPORTED_VERSION}. Skip the check.", + f"Version {schema_version} is not valid according to definition setting {definition_setting_expr}. Skip the check.", ) + return False + + return True + + +def execute_checker( + checker: types.ModuleType, + checker_data: models.CheckerData, + version_required: bool = True, +) -> None: + # Register checker + checker_data.result.register_checker( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=checker.CHECKER_ID, + description=checker.CHECKER_DESCRIPTION, + ) + + # Register rule uid + checker_data.result.register_rule_by_uid( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=checker.CHECKER_ID, + rule_uid=checker.RULE_UID, + ) + + # Check preconditions. If not satisfied then set status as SKIPPED and return + satisfied_preconditions = check_preconditions(checker, checker_data) + if not satisfied_preconditions: + return + + # Check definition setting and applicable version + if version_required: + satisfied_version = check_version(checker, checker_data) + if not satisfied_version: return # Execute checker @@ -161,9 +226,7 @@ def run_checks(config: Configuration, result: Result) -> None: ) # 1. Run basic checks - execute_checker( - basic.valid_xml_document, checker_data, required_definition_setting=False - ) + execute_checker(basic.valid_xml_document, checker_data, version_required=False) # Get xml root if the input file is a valid xml doc if result.all_checkers_completed_without_issue( @@ -173,15 +236,9 @@ def run_checks(config: Configuration, result: Result) -> None: checker_data.xml_file_path ) - execute_checker( - basic.root_tag_is_opendrive, checker_data, required_definition_setting=False - ) - execute_checker( - basic.fileheader_is_present, checker_data, required_definition_setting=False - ) - execute_checker( - basic.version_is_defined, checker_data, required_definition_setting=False - ) + execute_checker(basic.root_tag_is_opendrive, checker_data, version_required=False) + execute_checker(basic.fileheader_is_present, checker_data, version_required=False) + execute_checker(basic.version_is_defined, checker_data, version_required=False) # Get schema version if it exists if result.all_checkers_completed_without_issue( diff --git a/qc_opendrive/version.py b/qc_opendrive/version.py new file mode 100644 index 0000000..a255b23 --- /dev/null +++ b/qc_opendrive/version.py @@ -0,0 +1,77 @@ +# SPDX-License-Identifier: MPL-2.0 +# Copyright 2024, ASAM e.V. +# This Source Code Form is subject to the terms of the Mozilla +# Public License, v. 2.0. If a copy of the MPL was not distributed +# with this file, You can obtain one at https://mozilla.org/MPL/2.0/. + +import re +from typing import List, Optional +from semver.version import Version + + +def _is_lower_bound(expression: str) -> bool: + pattern = r"^(>=|>)" + match = re.match(pattern, expression) + if match: + return True + else: + return False + + +def _get_version_clause(applicable_version: str) -> bool: + version_clauses = applicable_version.split(",") + version_clauses = [clause.replace(" ", "") for clause in version_clauses] + return version_clauses + + +def is_valid_version_expression(version_expression: str) -> bool: + version_clauses = _get_version_clause(version_expression) + pattern = r"^(>=|<=|>|<)(\d+)\.(\d+)\.(\d+)$" + for clause in version_clauses: + match = re.match(pattern, clause) + if not match: + return False + + return True + + +def has_lower_bound(applicable_version: str) -> bool: + """ + Check if there is at least one lower bound in an applicable version string. + Example: + "<1.0.0,>0.0.1" returns True + "<1.0.0" returns False + """ + expressions = applicable_version.split(",") + + for expr in expressions: + if _is_lower_bound(expr): + return True + + return False + + +def match(version: str, applicable_version: str) -> bool: + """ + Check if the version is valid, given an applicable version. + + Applicable version is comma separated. The comma acts as a logical AND. + A candidate version must match all given version clauses in order to match + the applicable_version as a whole. + + The validity check follows the concept of Python version specifiers. + See: https://packaging.python.org/en/latest/specifications/version-specifiers/#id5 + + :param version: The version to be checked. + :param applicable_version: Comma separated applicable version. + """ + version_clauses = _get_version_clause(applicable_version) + + parsed_version = Version.parse(version) + + for clause in version_clauses: + is_matched = parsed_version.match(clause) + if not is_matched: + return False + + return True diff --git a/tests/data/junctions_connection_one_connection_element/junctions_connection_one_connection_element_v1_6_0_skipped.xodr b/tests/data/junctions_connection_one_connection_element/junctions_connection_one_connection_element_v1_6_0_skipped.xodr new file mode 100755 index 0000000..0f1a5a4 --- /dev/null +++ b/tests/data/junctions_connection_one_connection_element/junctions_connection_one_connection_element_v1_6_0_skipped.xodr @@ -0,0 +1,410 @@ + + +
+ + + +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ + + + + + + +
+ + + + + + + + + + + + + +
+
+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ + + + + + + +
+ + + + + + + + + + + + + +
+
+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ + + + + + + +
+ + + + + + + + + + + + + +
+
+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ + + + + + + +
+ + + + + + + + + + + + + +
+
+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ + + +
+ + + + + + + + + + + + +
+
+ + + +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ + + +
+ + + + + + + + + + + + +
+
+ + + +
+ + + + + + + + + + + + + + +
diff --git a/tests/test_semantic_checks.py b/tests/test_semantic_checks.py index 7ffeafa..304b51f 100644 --- a/tests/test_semantic_checks.py +++ b/tests/test_semantic_checks.py @@ -496,9 +496,10 @@ def test_junctions_connection_one_connection_element( "target_file,issue_count,issue_xpath", [ ("v1_8_0_valid", 0, []), + ("v1_6_0_skipped", 0, []), ], ) -def test_junctions_connection_one_connection_element_last_supported_version( +def test_junctions_connection_one_connection_element_applicable_version( target_file: str, issue_count: int, issue_xpath: List[str], diff --git a/tests/test_version.py b/tests/test_version.py new file mode 100644 index 0000000..ac814dc --- /dev/null +++ b/tests/test_version.py @@ -0,0 +1,66 @@ +# SPDX-License-Identifier: MPL-2.0 +# Copyright 2024, ASAM e.V. +# This Source Code Form is subject to the terms of the Mozilla +# Public License, v. 2.0. If a copy of the MPL was not distributed +# with this file, You can obtain one at https://mozilla.org/MPL/2.0/. + +import pytest +from qc_opendrive import version as qc_version + + +@pytest.mark.parametrize( + "applicable_version,has_lower_bound", + [ + (">1.0.0", True), + (">1.0.0,<2.0.0", True), + (">1.0.0,<=2.0.0", True), + (">=1.0.0", True), + (">=1.0.0,<2.0.0", True), + (">=1.0.0,<=2.0.0", True), + (">=1.0.0,>=1.0.1,<=2.0.0", True), + (">=1.0.0,<=1.0.1,<=2.0.0", True), + ("<=2.0.0", False), + ("<2.0.0", False), + ("<=2.0.0,<=3.0.0", False), + ("<2.0.0,<3.0.0", False), + ], +) +def test_has_lower_bound(applicable_version: str, has_lower_bound: bool) -> None: + assert qc_version.has_lower_bound(applicable_version) == has_lower_bound + + +@pytest.mark.parametrize( + "version,applicable_version,match", + [ + ("1.7.0", ">=1.7.0", True), + ("1.7.0", "<=1.7.0", True), + ("1.7.0", ">1.7.0", False), + ("1.7.0", "<1.7.0", False), + ("1.7.0", "<1.7.0,<1.8.0", False), + ("1.7.0", ">1.7.0,>1.6.0", False), + ("1.7.0", ">=1.7.0,>1.6.0", True), + ("1.7.0", ">=1.7.0,>1.8.0", False), + ("1.7.0", "<=1.7.0,>1.8.0", False), + ("1.7.0", "<=1.7.0,<1.8.0", True), + ("1.7.0", "<=1.7.0,<1.6.0", False), + ], +) +def test_match(version: str, applicable_version: str, match: bool) -> None: + assert qc_version.match(version, applicable_version) == match + + +@pytest.mark.parametrize( + "version_expression,is_valid", + [ + ("1.7.0", False), + (">1.7.0", True), + (">=1.7.0", True), + ("<1.7.0", True), + ("<=1.7.0", True), + ("==1.7.0", False), + ("!=1.7.0", False), + ("<1.7", False), + ], +) +def test_is_valid_version_expression(version_expression: str, is_valid: bool) -> None: + assert qc_version.is_valid_version_expression(version_expression) == is_valid From 0d18e5c9d2ae345edcf7f5196dde7cb3770c4f81 Mon Sep 17 00:00:00 2001 From: hoangtungdinh <11166240+hoangtungdinh@users.noreply.github.com> Date: Thu, 7 Nov 2024 16:34:25 +0100 Subject: [PATCH 3/4] Address reviews Signed-off-by: hoangtungdinh <11166240+hoangtungdinh@users.noreply.github.com> --- README.md | 34 +++++++++--------- qc_opendrive/main.py | 21 +++++------ qc_opendrive/version.py | 77 ++++++++++++++++++++++++++--------------- tests/test_version.py | 2 ++ 4 files changed, 76 insertions(+), 58 deletions(-) diff --git a/README.md b/README.md index 6e4d76c..2a789f0 100644 --- a/README.md +++ b/README.md @@ -285,7 +285,7 @@ Contributions of valid and invalid OpenDrive sample files are also welcome. New | `CHECKER_DESCRIPTION` | Required | The description of the checker | | `CHECKER_PRECONDITIONS` | Required | A set of other checkers in which if any of them raise an issue, the current checker will be skipped | | `RULE_UID` | Required | The rule UID of the rule that the checker will check | -| `APPLICABLE_VERSION` | Optional | An optional variable to define extra constraints on the applicable version. See details below. | +| `APPLICABLE_VERSIONS` | Optional | An optional variable to define extra constraints on the applicable version. See details below. | 1. Implement the checker logic in the following function: @@ -306,39 +306,39 @@ def run_checks(config: Configuration, result: Result) -> None: All the checkers in this checker bundle are implemented in this way. Take a look at some of them before implementing your first checker. -**A note on `APPLICABLE_VERSION`** +**A note on `APPLICABLE_VERSIONS`** -The `APPLICABLE_VERSION` variable can be used to define additional constraints on the versions of the input files that a rule supports, in addition to the **definition setting** in the rule UID. It can be specified in the same way as the [Python Version Specifiers](https://packaging.python.org/en/latest/specifications/version-specifiers/#id5). For example: +The `APPLICABLE_VERSIONS` variable can be used to define additional constraints on the versions of the input files that a rule supports, in addition to the **definition setting** in the rule UID. It can be specified in the same way as the [Python Version Specifiers](https://packaging.python.org/en/latest/specifications/version-specifiers/#id5). For example: ```python -APPLICABLE_VERSION = "<1.8.0" -APPLICABLE_VERSION = ">=1.6.0" -APPLICABLE_VERSION = "<1.8.0,>=1.6.0" +APPLICABLE_VERSIONS = "<1.8.0" +APPLICABLE_VERSIONS = ">=1.6.0" +APPLICABLE_VERSIONS = "<1.8.0,>=1.6.0" ``` -The specification consists of a series of version clauses, separated by commas. The comma is equivalent to a logical "AND" operator: a candidate version must match all given version clauses in order to match the **applicable version** as a whole. +The specification consists of a series of version clauses, separated by commas. The comma is equivalent to a logical "AND" operator: a candidate version must match all given version clauses in order to match the **applicable versions** as a whole. -The **applicable version** only supports versions of the form `major.minor.patch`. For example, `1.7.0rc1` is not supported, but `1.7.0` is supported. +The **applicable versions** only supports versions of in the full semantic form `major.minor.patch`. Elision of `minor` or `patch` elements are not supported. For example, `1.7.0rc1` and `1.7` are not supported, but `1.7.0` is supported. -The **applicable version** only supports the following comparison operators. +The **applicable versions** only supports the following comparison operators. * `<` smaller than **(upper bound)** * `<=` smaller or equal than **(upper bound)** * `>` greater than **(lower bound)** * `>=` greater or equal than **(lower bound)** -The **definition setting** in rule UID and the **applicable version** together define the versions of the input file in which a rule can be applied. For example, let's consider a rule UID for ASAM OpenDRIVE `asam.net:xodr:1.6.0:*`. The **definition setting** in this case is `1.6.0`. +The **definition setting** in rule UID and the **applicable versions** together define the versions of the input file in which a rule can be applied. For example, let's consider a rule UID for ASAM OpenDRIVE `asam.net:xodr:1.6.0:*`. The **definition setting** in this case is `1.6.0`. -1. If no **applicable version** is specified, a rule will be applied starting from the **definition setting** version, up to the most recent one. - * For the example, if the `APPLICABLE_VERSION` variable does not exist, then the rule is applied to OpenDRIVE versions 1.6.0, 1.6.1, 1.7.0, 1.8.0: the internal representation of the version specifier is `>=1.6.0`. +1. If no **applicable versions** is specified, a rule will be applied starting from the **definition setting** version, up to the most recent one. + * For the example, if the `APPLICABLE_VERSIONS` variable does not exist, then the rule is applied to OpenDRIVE versions 1.6.0, 1.6.1, 1.7.0, 1.8.0: the internal representation of the version specifier is `>=1.6.0`. -2. If the **applicable version** is specified, and defines only **upper bounds**, then the **definition setting** defines the lower bound - * For the example, if `APPLICABLE_VERSION = "<1.8.0"`, then the rule is applied to OpenDRIVE versions 1.6.0, 1.6.1, 1.7.0: the internal representation of the version specifier is `>=1.6.0,<1.8.0`. +2. If the **applicable versions** is specified, and defines only **upper bounds**, then the **definition setting** defines the lower bound + * For the example, if `APPLICABLE_VERSIONS = "<1.8.0"`, then the rule is applied to OpenDRIVE versions 1.6.0, 1.6.1, 1.7.0: the internal representation of the version specifier is `>=1.6.0,<1.8.0`. -3. If the **applicable version** is specified, and defines at least one **lower bound**, then the **definition setting** is ignored. Only the **lower bounds** defined in the **applicable version** are taken into account. - * For the example, if `APPLICABLE_VERSION = ">=1.5.0"`, then the rule is applied to OpenDRIVE versions 1.5.0, 1.6.0, 1.6.1, 1.7.0, 1.8.0: the internal representation of the version specifier is `>=1.5.0`. +3. If the **applicable versions** is specified, and defines at least one **lower bound**, then the **definition setting** is ignored. Only the **lower bounds** defined in the **applicable versions** are taken into account. + * For the example, if `APPLICABLE_VERSIONS = ">=1.5.0"`, then the rule is applied to OpenDRIVE versions 1.5.0, 1.6.0, 1.6.1, 1.7.0, 1.8.0: the internal representation of the version specifier is `>=1.5.0`. -| Case Number | Example Rule | `APPLICABLE_VERSION` | Internal Representation | File versions to be checked | +| Case Number | Example Rule | `APPLICABLE_VERSIONS` | Internal Representation | File versions to be checked | |-------------|-------------------------|----------------------|----------------------------------|-----------------------------------| | 1 | `asam.net:xodr:1.6.0:*` | `""` | `">=1.6.0"` | 1.6.0, 1.6.1, 1.7.0, 1.8.0 | | 2 | `asam.net:xodr:1.6.0:*` | `"<1.8.0"` | `">=1.6.0,<1.8.0"` | 1.6.0, 1.6.1, 1.7.0 | diff --git a/qc_opendrive/main.py b/qc_opendrive/main.py index 6e3a7eb..89dd98c 100644 --- a/qc_opendrive/main.py +++ b/qc_opendrive/main.py @@ -9,6 +9,7 @@ import types from qc_baselib import Configuration, Result, StatusType +from qc_baselib.models.result import RuleType from qc_opendrive import constants from qc_opendrive import version @@ -69,20 +70,14 @@ def check_version(checker: types.ModuleType, checker_data: models.CheckerData) - """ schema_version = checker_data.schema_version - splitted_rule_uid = checker.RULE_UID.split(":") - if len(splitted_rule_uid) != 4: - raise RuntimeError(f"Invalid rule uid: {checker.RULE_UID}") - - definition_setting = splitted_rule_uid[2] - definition_setting_expr = f">={definition_setting}" + rule_uid = RuleType(rule_uid=checker.RULE_UID) + definition_setting_expr = f">={rule_uid.definition_setting}" match_definition_setting = version.match(schema_version, definition_setting_expr) - applicable_version = ( - checker.APPLICABLE_VERSION if hasattr(checker, "APPLICABLE_VERSION") else None - ) + applicable_version = getattr(checker, "APPLICABLE_VERSION", "") # Check whether applicable version specification is valid - if applicable_version is not None and not version.is_valid_version_expression( + if applicable_version and not version.is_valid_version_expression( applicable_version ): checker_data.result.set_checker_status( @@ -110,14 +105,14 @@ def check_version(checker: types.ModuleType, checker_data: models.CheckerData) - checker_data.result.add_checker_summary( constants.BUNDLE_NAME, checker.CHECKER_ID, - f"The definition setting {definition_setting} is not valid. Skip the check.", + f"The definition setting {rule_uid.definition_setting} is not valid. Skip the check.", ) return False match_applicable_version = ( version.match(schema_version, applicable_version) - if applicable_version is not None + if applicable_version else True ) @@ -138,7 +133,7 @@ def check_version(checker: types.ModuleType, checker_data: models.CheckerData) - return False # Check definition setting if there is no applicable version or applicable version has no lower bound - if applicable_version is None or not version.has_lower_bound(applicable_version): + if not version.has_lower_bound(applicable_version): if not match_definition_setting: checker_data.result.set_checker_status( checker_bundle_name=constants.BUNDLE_NAME, diff --git a/qc_opendrive/version.py b/qc_opendrive/version.py index a255b23..5061b20 100644 --- a/qc_opendrive/version.py +++ b/qc_opendrive/version.py @@ -5,50 +5,52 @@ # with this file, You can obtain one at https://mozilla.org/MPL/2.0/. import re -from typing import List, Optional +from typing import List from semver.version import Version +_is_lower_bound_pattern = re.compile(r"^>") + + def _is_lower_bound(expression: str) -> bool: - pattern = r"^(>=|>)" - match = re.match(pattern, expression) - if match: - return True - else: - return False + return bool(_is_lower_bound_pattern.match(expression)) -def _get_version_clause(applicable_version: str) -> bool: - version_clauses = applicable_version.split(",") - version_clauses = [clause.replace(" ", "") for clause in version_clauses] - return version_clauses +_re_split_clauses = re.compile(r"\s*,\s*") +_re_remove_spaces = re.compile(r"\s+") -def is_valid_version_expression(version_expression: str) -> bool: - version_clauses = _get_version_clause(version_expression) - pattern = r"^(>=|<=|>|<)(\d+)\.(\d+)\.(\d+)$" - for clause in version_clauses: - match = re.match(pattern, clause) - if not match: - return False +def _get_version_clauses(applicable_versions: str) -> List[str]: + version_clauses = _re_split_clauses.split(applicable_versions) + return [_re_remove_spaces.sub("", vc) for vc in version_clauses] - return True + +_is_valid_clause_pattern = re.compile(r"^([<>]=?)(\d+)\.(\d+)\.(\d+)$") -def has_lower_bound(applicable_version: str) -> bool: +def _is_valid_clause(clause: str) -> bool: + return bool(_is_valid_clause_pattern.match(clause)) + + +def is_valid_version_expression(version_expression: str) -> bool: + return all( + _is_valid_clause(clause) for clause in _get_version_clauses(version_expression) + ) + + +def has_lower_bound(applicable_versions: str) -> bool: """ Check if there is at least one lower bound in an applicable version string. Example: "<1.0.0,>0.0.1" returns True "<1.0.0" returns False """ - expressions = applicable_version.split(",") - - for expr in expressions: - if _is_lower_bound(expr): - return True - - return False + return any( + ( + _is_lower_bound(clause) + for clause in _get_version_clauses(applicable_versions) + ) + ) def match(version: str, applicable_version: str) -> bool: @@ -65,7 +67,7 @@ def match(version: str, applicable_version: str) -> bool: :param version: The version to be checked. :param applicable_version: Comma separated applicable version. """ - version_clauses = _get_version_clause(applicable_version) + version_clauses = _get_version_clauses(applicable_version) parsed_version = Version.parse(version) @@ -75,3 +77,22 @@ def match(version: str, applicable_version: str) -> bool: return False return True + + +def match(version: str, applicable_versions: str) -> bool: + """ + Check if the version is valid, given an applicable version. + Applicable version is comma separated. The comma acts as a logical AND. + A candidate version must match all given version clauses in order to match + the applicable_version as a whole. + The validity check follows the concept of Python version specifiers. + See: https://packaging.python.org/en/latest/specifications/version-specifiers/#id5 + + :param version: The version to be checked. + :param applicable_version: Comma separated applicable version. Invalid version clauses will force the check to fail + :return: a boolean for the match + """ + version = Version.parse(version) + clauses = _get_version_clauses(applicable_versions) + + return all(_is_valid_clause(clause) and version.match(clause) for clause in clauses) diff --git a/tests/test_version.py b/tests/test_version.py index ac814dc..78262b7 100644 --- a/tests/test_version.py +++ b/tests/test_version.py @@ -23,6 +23,7 @@ ("<2.0.0", False), ("<=2.0.0,<=3.0.0", False), ("<2.0.0,<3.0.0", False), + ("", False), ], ) def test_has_lower_bound(applicable_version: str, has_lower_bound: bool) -> None: @@ -43,6 +44,7 @@ def test_has_lower_bound(applicable_version: str, has_lower_bound: bool) -> None ("1.7.0", "<=1.7.0,>1.8.0", False), ("1.7.0", "<=1.7.0,<1.8.0", True), ("1.7.0", "<=1.7.0,<1.6.0", False), + ("1.7.0", "<=1.7.0,<1.8", False), ], ) def test_match(version: str, applicable_version: str, match: bool) -> None: From d27f4146dd09237743721dbf6b2a71af5b16e0cb Mon Sep 17 00:00:00 2001 From: hoangtungdinh <11166240+hoangtungdinh@users.noreply.github.com> Date: Fri, 8 Nov 2024 11:21:40 +0100 Subject: [PATCH 4/4] Remove redundant function and clean up Signed-off-by: hoangtungdinh <11166240+hoangtungdinh@users.noreply.github.com> --- qc_opendrive/main.py | 12 ++---------- qc_opendrive/version.py | 33 ++++----------------------------- tests/test_version.py | 2 ++ 3 files changed, 8 insertions(+), 39 deletions(-) diff --git a/qc_opendrive/main.py b/qc_opendrive/main.py index 89dd98c..502fa45 100644 --- a/qc_opendrive/main.py +++ b/qc_opendrive/main.py @@ -77,9 +77,7 @@ def check_version(checker: types.ModuleType, checker_data: models.CheckerData) - applicable_version = getattr(checker, "APPLICABLE_VERSION", "") # Check whether applicable version specification is valid - if applicable_version and not version.is_valid_version_expression( - applicable_version - ): + if not version.is_valid_version_expression(applicable_version): checker_data.result.set_checker_status( checker_bundle_name=constants.BUNDLE_NAME, checker_id=checker.CHECKER_ID, @@ -110,14 +108,8 @@ def check_version(checker: types.ModuleType, checker_data: models.CheckerData) - return False - match_applicable_version = ( - version.match(schema_version, applicable_version) - if applicable_version - else True - ) - # First, check applicable version - if not match_applicable_version: + if not version.match(schema_version, applicable_version): checker_data.result.set_checker_status( checker_bundle_name=constants.BUNDLE_NAME, checker_id=checker.CHECKER_ID, diff --git a/qc_opendrive/version.py b/qc_opendrive/version.py index 5061b20..07b5157 100644 --- a/qc_opendrive/version.py +++ b/qc_opendrive/version.py @@ -22,14 +22,15 @@ def _is_lower_bound(expression: str) -> bool: def _get_version_clauses(applicable_versions: str) -> List[str]: version_clauses = _re_split_clauses.split(applicable_versions) - return [_re_remove_spaces.sub("", vc) for vc in version_clauses] + version_clauses = [_re_remove_spaces.sub("", vc) for vc in version_clauses] + return [vc for vc in version_clauses if vc != ""] -_is_valid_clause_pattern = re.compile(r"^([<>]=?)(\d+)\.(\d+)\.(\d+)$") +_re_is_valid_clause = re.compile(r"^([<>]=?)(\d+)\.(\d+)\.(\d+)$") def _is_valid_clause(clause: str) -> bool: - return bool(_is_valid_clause_pattern.match(clause)) + return bool(_re_is_valid_clause.match(clause)) def is_valid_version_expression(version_expression: str) -> bool: @@ -53,32 +54,6 @@ def has_lower_bound(applicable_versions: str) -> bool: ) -def match(version: str, applicable_version: str) -> bool: - """ - Check if the version is valid, given an applicable version. - - Applicable version is comma separated. The comma acts as a logical AND. - A candidate version must match all given version clauses in order to match - the applicable_version as a whole. - - The validity check follows the concept of Python version specifiers. - See: https://packaging.python.org/en/latest/specifications/version-specifiers/#id5 - - :param version: The version to be checked. - :param applicable_version: Comma separated applicable version. - """ - version_clauses = _get_version_clauses(applicable_version) - - parsed_version = Version.parse(version) - - for clause in version_clauses: - is_matched = parsed_version.match(clause) - if not is_matched: - return False - - return True - - def match(version: str, applicable_versions: str) -> bool: """ Check if the version is valid, given an applicable version. diff --git a/tests/test_version.py b/tests/test_version.py index 78262b7..d01ccd0 100644 --- a/tests/test_version.py +++ b/tests/test_version.py @@ -45,6 +45,7 @@ def test_has_lower_bound(applicable_version: str, has_lower_bound: bool) -> None ("1.7.0", "<=1.7.0,<1.8.0", True), ("1.7.0", "<=1.7.0,<1.6.0", False), ("1.7.0", "<=1.7.0,<1.8", False), + ("1.7.0", "", True), ], ) def test_match(version: str, applicable_version: str, match: bool) -> None: @@ -62,6 +63,7 @@ def test_match(version: str, applicable_version: str, match: bool) -> None: ("==1.7.0", False), ("!=1.7.0", False), ("<1.7", False), + ("", True), ], ) def test_is_valid_version_expression(version_expression: str, is_valid: bool) -> None: