From e3a249e76354f003a0a9bc179016b508b84355e9 Mon Sep 17 00:00:00 2001 From: Marco Favorito Date: Tue, 6 Jun 2023 22:17:31 +0200 Subject: [PATCH 01/51] fix: remove constants.py module and use pddl/parser/symbols.py --- pddl/_validation.py | 4 ++-- pddl/constants.py | 16 ---------------- pddl/parser/domain.py | 3 +-- pddl/parser/symbols.py | 2 ++ tests/test_domain.py | 4 ++-- 5 files changed, 7 insertions(+), 22 deletions(-) delete mode 100644 pddl/constants.py diff --git a/pddl/_validation.py b/pddl/_validation.py index 700590b4..2ba87124 100644 --- a/pddl/_validation.py +++ b/pddl/_validation.py @@ -14,12 +14,12 @@ from typing import Collection, Dict, Optional, Set, Tuple -from pddl.constants import OBJECT from pddl.custom_types import name, to_names # noqa: F401 from pddl.exceptions import PDDLValidationError from pddl.helpers.base import find_cycle from pddl.logic import Constant, Predicate from pddl.logic.terms import Term +from pddl.parser.symbols import Symbols def _check_types_dictionary(type_dict: Dict[name, Optional[name]]) -> None: @@ -49,7 +49,7 @@ def _check_types_dictionary(type_dict: Dict[name, Optional[name]]) -> None: return # check `object` type - object_name = name(OBJECT) + object_name = name(Symbols.OBJECT.value) if object_name in type_dict and type_dict[object_name] is not None: object_supertype = type_dict[object_name] raise PDDLValidationError( diff --git a/pddl/constants.py b/pddl/constants.py deleted file mode 100644 index 2668d5c7..00000000 --- a/pddl/constants.py +++ /dev/null @@ -1,16 +0,0 @@ -# -# Copyright 2021-2023 WhiteMech -# -# ------------------------------ -# -# This file is part of pddl. -# -# Use of this source code is governed by an MIT-style -# license that can be found in the LICENSE file or at -# https://opensource.org/licenses/MIT. -# - -"""Module for library-wide constants, e.g. keywords.""" - -EITHER = "either" -OBJECT = "object" diff --git a/pddl/parser/domain.py b/pddl/parser/domain.py index 880fda7a..1ce703c6 100644 --- a/pddl/parser/domain.py +++ b/pddl/parser/domain.py @@ -16,7 +16,6 @@ from lark import Lark, ParseError, Transformer -from pddl.constants import EITHER from pddl.core import Action, Domain, Requirements from pddl.exceptions import PDDLMissingRequirementError, PDDLParsingError from pddl.helpers.base import assert_, safe_index @@ -409,7 +408,7 @@ def _process_type_def(self, type_def: List[str]) -> Set[str]: # if we are here, type_def is of the form (either t1 ... tn) either_keyword, types = type_def[0], type_def[1:] - assert_(str(either_keyword) == EITHER) + assert_(str(either_keyword) == Symbols.EITHER.value) return set(types) diff --git a/pddl/parser/symbols.py b/pddl/parser/symbols.py index 4a409c4c..f5c2acc0 100644 --- a/pddl/parser/symbols.py +++ b/pddl/parser/symbols.py @@ -32,6 +32,8 @@ class Symbols(Enum): FORALL = "forall" EXISTS = "exists" WHEN = "when" + OBJECT = "object" + EITHER = "either" DERIVED = ":derived" DOMAIN_P = ":domain" OBJECTS = ":objects" diff --git a/tests/test_domain.py b/tests/test_domain.py index d45b9439..582a90da 100644 --- a/tests/test_domain.py +++ b/tests/test_domain.py @@ -17,13 +17,13 @@ import pytest -from pddl.constants import OBJECT from pddl.core import Action, Domain from pddl.exceptions import PDDLValidationError from pddl.logic import Constant, Variable from pddl.logic.base import Not, TrueFormula from pddl.logic.helpers import constants, variables from pddl.logic.predicates import DerivedPredicate, Predicate +from pddl.parser.symbols import Symbols from tests.conftest import pddl_objects_domains @@ -94,7 +94,7 @@ def test_cycles_in_type_defs_not_allowed() -> None: def test_object_must_not_be_subtype() -> None: """Test that when the `object` type is used as subtype we raise error.""" my_type = "my_type" - type_set = {OBJECT: my_type} + type_set = {Symbols.OBJECT.value: my_type} with pytest.raises( PDDLValidationError, From 658dc62608e05fe4d0a2ecbb7006970cadb90ec1 Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 10:06:11 +0200 Subject: [PATCH 02/51] fix: add missing __invert__ method for True/FalseFormula classes --- pddl/logic/base.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pddl/logic/base.py b/pddl/logic/base.py index 678baaae..1b2c6448 100644 --- a/pddl/logic/base.py +++ b/pddl/logic/base.py @@ -134,6 +134,10 @@ def __hash__(self): """Hash the object.""" return hash(TrueFormula) + def __invert__(self) -> Formula: + """Negate the formula.""" + return FALSE + def __neg__(self) -> Formula: """Negate.""" return FALSE @@ -158,6 +162,10 @@ def __hash__(self): """Hash the object.""" return hash(FalseFormula) + def __invert__(self) -> Formula: + """Negate the formula.""" + return TRUE + def __neg__(self) -> Formula: """Negate.""" return TRUE From d7d05344bcb499c6aafb57a9293501cb5a91a40b Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 10:06:54 +0200 Subject: [PATCH 03/51] test: fix blocksworld_fond/p01.pddl goal The goal was just '(and )', restore the original reachability goal. --- tests/fixtures/code_objects/blocksworld_fond.py | 2 +- tests/fixtures/pddl_files/blocksworld_fond/p01.pddl | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/tests/fixtures/code_objects/blocksworld_fond.py b/tests/fixtures/code_objects/blocksworld_fond.py index 8b125bf6..06683ab4 100644 --- a/tests/fixtures/code_objects/blocksworld_fond.py +++ b/tests/fixtures/code_objects/blocksworld_fond.py @@ -108,7 +108,7 @@ def blocksworld_fond_01(): clear(Table), } - goal = And() + goal = on(B, A) problem_name = "sussman-anomaly" diff --git a/tests/fixtures/pddl_files/blocksworld_fond/p01.pddl b/tests/fixtures/pddl_files/blocksworld_fond/p01.pddl index 38dfc256..a8f0c549 100644 --- a/tests/fixtures/pddl_files/blocksworld_fond/p01.pddl +++ b/tests/fixtures/pddl_files/blocksworld_fond/p01.pddl @@ -4,8 +4,5 @@ (:init (block A) (block B) (block C) (block Table) (on C A) (on A Table) (on B Table) (clear C) (clear B) (clear Table)) - ;(:goal (eventually (on B A)) - ;) - (:goal (and )) - + (:goal (on B A)) ) From d5b509cea6688698824736598eddbc81acd92ac2 Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 10:10:38 +0200 Subject: [PATCH 04/51] chore: add 'check' function to generalize assert_ for any exception type --- pddl/helpers/base.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pddl/helpers/base.py b/pddl/helpers/base.py index 09e85230..fe0806ac 100644 --- a/pddl/helpers/base.py +++ b/pddl/helpers/base.py @@ -44,8 +44,15 @@ def assert_(condition: bool, message: str = "") -> None: when the code is compiled in optimized mode. For more information, see https://bandit.readthedocs.io/en/1.7.5/plugins/b101_assert_used.html """ + check(condition, message=message, exception_cls=AssertionError) + + +def check( + condition: bool, message: str = "", exception_cls: Type[Exception] = AssertionError +) -> None: + """Check a condition, and if false, raise exception.""" if not condition: - raise AssertionError(message) + raise exception_cls(message) def ensure(arg: Optional[Any], default: Any): From 3c53e3f30d5feca72cffa35cebe5c6b12f52d211 Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 10:11:31 +0200 Subject: [PATCH 05/51] feat: add TypesIndex class This commit adds the parsing utility class "TypesIndex". TypesIndex is an index for PDDL types and PDDL names/variables. It is used to index PDDL names and variables by their types. OrderedDict is used to preserve the order of the types and the names, e.g. for predicate variables. Other types of validations are performed to ensure that the types index is consistent. In this commit, it is used only to parse typed lists of names (not variables). --- pddl/parser/types_index.py | 160 +++++++++++++++++++++++++++++++++++++ 1 file changed, 160 insertions(+) create mode 100644 pddl/parser/types_index.py diff --git a/pddl/parser/types_index.py b/pddl/parser/types_index.py new file mode 100644 index 00000000..b9b1f6fa --- /dev/null +++ b/pddl/parser/types_index.py @@ -0,0 +1,160 @@ +# +# Copyright 2021-2023 WhiteMech +# +# ------------------------------ +# +# This file is part of pddl. +# +# Use of this source code is governed by an MIT-style +# license that can be found in the LICENSE file or at +# https://opensource.org/licenses/MIT. +# + +"""Utility to handle typed lists.""" +import itertools +from collections import OrderedDict +from typing import Dict, List, Optional +from typing import OrderedDict as OrderedDictType +from typing import Set, Union, cast + +from pddl.helpers.base import check, safe_index +from pddl.parser.symbols import Symbols + + +class TypesIndex: + """ + An index for PDDL types and PDDL names/variables. + + This class is used to index PDDL names and variables by their types. + OrderedDict is used to preserve the order of the types and the names, e.g. for predicate variables. + + Other types of validations are performed to ensure that the types index is consistent. + """ + + def __init__(self) -> None: + """Initialize the types index.""" + self._types_to_items: OrderedDictType[str, Set[str]] = OrderedDict() + self._item_to_types: OrderedDictType[str, Set[str]] = OrderedDict() + + def add_item(self, item_name: str, type_tags: Set[str]) -> None: + """Add an item.""" + if item_name in self._item_to_types: + raise ValueError( + f"duplicate name '{item_name}' in typed list: already present with " + f"types {sorted(map(str,self._item_to_types[item_name]))}" + ) + + exisiting_tags = self._item_to_types.get(item_name, set()) + for type_tag in type_tags: + if type_tag in exisiting_tags: + raise ValueError( + f"duplicate type tag '{type_tag}' in typed list: type already specified for item {item_name}" + ) + + for type_tag in type_tags: + self._types_to_items.setdefault(type_tag, set()).add(item_name) + self._item_to_types.setdefault(item_name, set()).update(type_tags) + + def get_typed_list_of_names(self) -> Dict[str, Optional[str]]: + """Get the typed list of names in form of dictionary.""" + result: Dict[str, Optional[str]] = {} + for item, types_tags in self._item_to_types.items(): + if len(types_tags) > 1: + raise ValueError( + f"typed list names should not have more than one type, got '{item}' with " + f"types {sorted(map(str,types_tags))}" + ) + type_tag = next(iter(types_tags)) if len(types_tags) == 1 else None + result[item] = type_tag + return result + + @classmethod + def parse_typed_list(cls, tokens: List[Union[str, List[str]]]) -> "TypesIndex": + """ + Parse typed list. + + This method takes in input a list of tokens as returned by the domain or problem parsers, + and returns a TypesIndex object. + + The input list of tokens must have the following format: + - if the list is not typed, it is simply a list of names + - if the list is typed, the format is: [name_1, ..., name_n], "-", [type_1, ..., type_m], ... + + >>> index = TypesIndex.parse_typed_list(["a", "b", "c"]) + >>> index.get_typed_list_of_names() + {'a': None, 'b': None, 'c': None} + + >>> index = TypesIndex.parse_typed_list(["a", "b", "c", "-", ["t1"]]) + >>> index.get_typed_list_of_names() + {'a': 't1', 'b': 't1', 'c': 't1'} + + >>> index = TypesIndex.parse_typed_list(["a", "b", "c", "-", ["t1", "t2"]]) + >>> index.get_typed_list_of_names() + Traceback (most recent call last): + ... + ValueError: typed list names should not have more than one type, got 'a' with types ['t1', 't2'] + + :param tokens: the list of tokens + :return: the TypesIndex object + """ + result = TypesIndex() + + type_sep_index = safe_index(tokens, Symbols.TYPE_SEP.value) + + if type_sep_index is None: + # simple list of names + cls._add_typed_lists(result, 0, len(tokens), tokens, set()) + return result + + # if we are here, the matched pattern is: [name_1, ..., name_n], "-", parent_name, ... + # Consume typed sublists iteratively. Caveat: the last typed list *might* have no parent type. + + # the index of the separator symbol for the current typed sublist being processed + type_sep_index = safe_index(tokens, Symbols.TYPE_SEP.value) + # the index of the first element of the current typed sublist + start_index = 0 + # the index of the last element of the current typed sublist + end_index = len(tokens) + while type_sep_index is not None: + # the name of the parent type is the element after the separator + parent_names = tokens[type_sep_index + 1] + # handle the case of a single parent type + if not isinstance(parent_names, list): + parent_names = [parent_names] + + # parse the typed list + cls._add_typed_lists( + result, start_index, type_sep_index, tokens, set(parent_names) + ) + + # go to next typed list (if any) + start_index = type_sep_index + 2 + type_sep_index = safe_index( + tokens, Symbols.TYPE_SEP.value, start_index, end_index + ) + + # this is to handle the case the last sublist is not typed + if start_index != end_index: + # parse the last typed list, with no type. + cls._add_typed_lists(result, start_index, end_index, tokens, set()) + + return result + + @classmethod + def _add_typed_lists( + cls, + result: "TypesIndex", + start_index: int, + end_index: int, + tokens: List[Union[str, List[str]]], + type_tags: Set[str], + ) -> None: + """ + Merge typed lists. + + Side-effect on the 'result' dictionary. The start_index and end_index are needed to avoid useless + sublist copies. + """ + for name in itertools.islice(tokens, start_index, end_index): + check(isinstance(name, str), f"invalid item '{name}' in typed list") + result.add_item(cast(str, name), type_tags) From 3cf8cb450c9ec934486e19bba91be4192864ccc9 Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 10:18:56 +0200 Subject: [PATCH 06/51] feat: changing domain parser behaviour on typed_list_name (backward compatible) This commit changes the way typed_list_name rule is parsed. This does not change the parsing behaviour from the user perspective, but it is a non-functional change to improve the performances of the Lark parser. With the previous version of the rule 'typed_list_name: NAME+ TYPE_SEP primitive_type (typed_list_name)', the Lark parser internals would perform a recursive call whenever a typed_list_name is matched. This might cause an arbitrarily long stack of calls whenever the parser got an input like: ``` element1 - t1 element2 - t1 element3 t1 ... ``` The above list of tokens is parsed as: ``` element1 - t1 element2 - t1 element3 - t1 ... ``` For large inputs, this will easily hit the recursion depth limit. With the new approach, the entire list of tokens is parsed at once. This adds some complexity in the parsing function for the typed_list_name rule, but we have more control in how the parsing is performed; in particular, we can parse the tokens *iteratively* rather than *recursively*. Finally, the NAME* pattern is appended at the end of the typed_list_name rule. This is because, according to the syntax specification, the last sublist of names might be non-typed. The implementation exploits the newly added TypesIndex class which handles corner cases and syntax errors (e.g. duplicated entries in the list). --- pddl/parser/domain.lark | 2 +- pddl/parser/domain.py | 43 +++++++---------------------------------- 2 files changed, 8 insertions(+), 37 deletions(-) diff --git a/pddl/parser/domain.lark b/pddl/parser/domain.lark index 0520579a..09fed456 100644 --- a/pddl/parser/domain.lark +++ b/pddl/parser/domain.lark @@ -54,7 +54,7 @@ typed_list_variable: variable* ?variable: "?" NAME typed_list_name: NAME* - | NAME+ TYPE_SEP primitive_type (typed_list_name) + | (NAME+ TYPE_SEP primitive_type)+ NAME* type_def: LPAR EITHER primitive_type+ RPAR | primitive_type ?primitive_type: NAME diff --git a/pddl/parser/domain.py b/pddl/parser/domain.py index 1ce703c6..82b1c13e 100644 --- a/pddl/parser/domain.py +++ b/pddl/parser/domain.py @@ -34,6 +34,7 @@ from pddl.logic.terms import Constant, Variable from pddl.parser import DOMAIN_GRAMMAR_FILE, PARSERS_DIRECTORY from pddl.parser.symbols import Symbols +from pddl.parser.types_index import TypesIndex class DomainTransformer(Transformer): @@ -87,11 +88,11 @@ def requirements(self, args): def types(self, args): """Parse the 'types' rule.""" has_typing_requirement = self._has_requirement(Requirements.TYPING) - type_definition = args[2] - have_type_hierarchy = any(type_definition.values()) + types_definition = args[2] + have_type_hierarchy = any(types_definition.values()) if have_type_hierarchy and not has_typing_requirement: raise PDDLMissingRequirementError(Requirements.TYPING) - return dict(types=args[2]) + return dict(types=types_definition) def constants(self, args): """Process the 'constant_def' rule.""" @@ -294,39 +295,9 @@ def atomic_formula_skeleton(self, args): return Predicate(name, *variables) def typed_list_name(self, args) -> Dict[str, Optional[str]]: - """ - Process the 'typed_list_name' rule. - - Return a dictionary with as keys the names and as value the type of the name. - - Steps: - - if the '-' symbol is not present, then return the list of names - - if the '-' symbol is present, parse the names with their type tags - - :param args: the argument of this grammar rule - :return: a typed list (name) - """ - type_sep_index = safe_index(args, Symbols.TYPE_SEP.value) - - if type_sep_index is None: - # simple list of names - return self._parse_simple_typed_list(args, check_for_duplicates=True) - - # if we are here, the matched pattern is: [name_1, ..., name_n], "-", parent_name, other_typed_list_dict - # make sure there are only two tokens after "-" - assert_(len(args[type_sep_index:]) == 3, "unexpected parser state") - - names: Tuple[str, ...] = tuple(args[:type_sep_index]) - parent_name: str = str(args[type_sep_index + 1]) - other_typed_list_dict: Mapping[str, Optional[str]] = args[type_sep_index + 2] - new_typed_list_dict: Mapping[str, Optional[str]] = { - obj: parent_name for obj in names - } - - # check type conflicts - self._check_duplicates(other_typed_list_dict.keys(), new_typed_list_dict.keys()) - - return {**new_typed_list_dict, **other_typed_list_dict} + """Process the 'typed_list_name' rule.""" + types_index = TypesIndex.parse_typed_list(args) + return types_index.get_typed_list_of_names() def typed_list_variable(self, args) -> Dict[str, Set[str]]: """ From cf25df5b8a33e9ce106b48b729176f65777e81da Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 10:23:33 +0200 Subject: [PATCH 07/51] test: add problem parsing in test_formatter.py --- tests/test_formatter.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/test_formatter.py b/tests/test_formatter.py index 8020e3d5..851ca6ea 100644 --- a/tests/test_formatter.py +++ b/tests/test_formatter.py @@ -16,8 +16,8 @@ import pytest -from pddl.formatter import domain_to_string -from tests.conftest import DOMAIN_FILES +from pddl.formatter import domain_to_string, problem_to_string +from tests.conftest import DOMAIN_FILES, PROBLEM_FILES @pytest.mark.parametrize("pddl_file", DOMAIN_FILES) @@ -27,3 +27,12 @@ def test_domain_formatter(domain_parser, pddl_file: Path): actual_domain_str = domain_to_string(expected_domain_obj) actual_domain_obj = domain_parser(actual_domain_str) assert actual_domain_obj == expected_domain_obj + + +@pytest.mark.parametrize("pddl_file", PROBLEM_FILES) +def test_problem_formatter(problem_parser, pddl_file): + """Test generated problem formatting.""" + expected_problem_obj = problem_parser(pddl_file.read_text()) + actual_problem_str = problem_to_string(expected_problem_obj) + actual_problem_obj = problem_parser(actual_problem_str) + assert actual_problem_obj == expected_problem_obj From ccb50970a6dd05aaf83b49787daf29933da3d1d7 Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 11:22:32 +0200 Subject: [PATCH 08/51] test: fix expected error messages when duplicated names/types occur --- pddl/parser/domain.py | 9 +++++++-- pddl/parser/types_index.py | 6 ++++-- tests/test_parser.py | 16 ++++++++-------- 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/pddl/parser/domain.py b/pddl/parser/domain.py index 82b1c13e..0ce83071 100644 --- a/pddl/parser/domain.py +++ b/pddl/parser/domain.py @@ -296,8 +296,13 @@ def atomic_formula_skeleton(self, args): def typed_list_name(self, args) -> Dict[str, Optional[str]]: """Process the 'typed_list_name' rule.""" - types_index = TypesIndex.parse_typed_list(args) - return types_index.get_typed_list_of_names() + try: + types_index = TypesIndex.parse_typed_list(args) + return types_index.get_typed_list_of_names() + except ValueError as e: + raise PDDLParsingError( + f"error while parsing tokens {list(map(str, args))}: {str(e)}" + ) from None def typed_list_variable(self, args) -> Dict[str, Set[str]]: """ diff --git a/pddl/parser/types_index.py b/pddl/parser/types_index.py index b9b1f6fa..54ae6f40 100644 --- a/pddl/parser/types_index.py +++ b/pddl/parser/types_index.py @@ -39,9 +39,11 @@ def __init__(self) -> None: def add_item(self, item_name: str, type_tags: Set[str]) -> None: """Add an item.""" if item_name in self._item_to_types: + types_list = sorted(map(str, self._item_to_types[item_name])) + types_list_str = f" with types {types_list}" if len(types_list) > 0 else "" raise ValueError( - f"duplicate name '{item_name}' in typed list: already present with " - f"types {sorted(map(str,self._item_to_types[item_name]))}" + f"duplicate name '{item_name}' in typed list already present" + + types_list_str ) exisiting_tags = self._item_to_types.get(item_name, set()) diff --git a/tests/test_parser.py b/tests/test_parser.py index a2e13321..c5182600 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -135,8 +135,8 @@ def test_types_repetition_in_simple_typed_lists_not_allowed() -> None: with pytest.raises( lark.exceptions.VisitError, - match="duplicate items \\['a'\\] found in the typed list: " - "\\['a', 'b', 'c', 'a'\\]", + match=".*error while parsing tokens \\['a', 'b', 'c', 'a'\\]: " + "duplicate name 'a' in typed list already present", ): DomainParser()(domain_str) @@ -154,8 +154,8 @@ def test_types_repetition_in_typed_lists_not_allowed() -> None: with pytest.raises( lark.exceptions.VisitError, - match="detected conflicting items in a typed list: items occurred " - "twice: \\['a'\\]", + match=".*error while parsing tokens \\['a', '-', 't1', 'b', 'c', '-', 't2', 'a', '-', 't3'\\]: " + "duplicate name 'a' in typed list already present with types \\['t1'\\]", ): DomainParser()(domain_str) @@ -174,8 +174,8 @@ def test_constants_repetition_in_simple_typed_lists_not_allowed() -> None: with pytest.raises( lark.exceptions.VisitError, - match="duplicate items \\['c1'\\] found in the typed list: " - "\\['c1', 'c2', 'c3', 'c1'\\]", + match=".*error while parsing tokens \\['c1', 'c2', 'c3', 'c1'\\]: " + "duplicate name 'c1' in typed list already present", ): DomainParser()(domain_str) @@ -194,7 +194,7 @@ def test_constants_repetition_in_typed_lists_not_allowed() -> None: with pytest.raises( lark.exceptions.VisitError, - match="detected conflicting items in a typed list: items occurred " - "twice: \\['c1'\\]", + match=".*error while parsing tokens \\['c1', '-', 't1', 'c1', '-', 't2'\\]: " + "duplicate name 'c1' in typed list already present with types \\['t1'\\]", ): DomainParser()(domain_str) From 7a7836c77437d1c0f33d20d94111eeb8846f4573 Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 11:50:30 +0200 Subject: [PATCH 09/51] fix: minor fix to object-supertypes-error --- pddl/_validation.py | 4 ++-- tests/test_domain.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pddl/_validation.py b/pddl/_validation.py index 2ba87124..24ee27f5 100644 --- a/pddl/_validation.py +++ b/pddl/_validation.py @@ -34,7 +34,7 @@ def _check_types_dictionary(type_dict: Dict[name, Optional[name]]) -> None: >>> _check_types_dictionary({name("object"): a}) Traceback (most recent call last): ... - pddl.exceptions.PDDLValidationError: object must not have supertypes, but got object is a subtype of a + pddl.exceptions.PDDLValidationError: object must not have supertypes, but got 'object' is a subtype of 'a' 3) If cycles in the type hierarchy graph are present, an error is raised: >>> a, b, c = to_names(["a", "b", "c"]) @@ -53,7 +53,7 @@ def _check_types_dictionary(type_dict: Dict[name, Optional[name]]) -> None: if object_name in type_dict and type_dict[object_name] is not None: object_supertype = type_dict[object_name] raise PDDLValidationError( - f"object must not have supertypes, but got object is a subtype of {object_supertype}" + f"object must not have supertypes, but got 'object' is a subtype of '{object_supertype}'" ) # check cycles diff --git a/tests/test_domain.py b/tests/test_domain.py index 582a90da..d8d35e92 100644 --- a/tests/test_domain.py +++ b/tests/test_domain.py @@ -98,7 +98,7 @@ def test_object_must_not_be_subtype() -> None: with pytest.raises( PDDLValidationError, - match=rf"object must not have supertypes, but got object is a subtype of {my_type}", + match=rf"object must not have supertypes, but got 'object' is a subtype of '{my_type}'", ): Domain("test", types=type_set) # type: ignore From f1e331685c8dd41549d253008380ed14c3f312c3 Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 11:51:18 +0200 Subject: [PATCH 10/51] fix: use 'name' type to validate tokens of typed list --- pddl/parser/domain.py | 3 +- pddl/parser/types_index.py | 100 ++++++++++++++++++++++++++----------- 2 files changed, 72 insertions(+), 31 deletions(-) diff --git a/pddl/parser/domain.py b/pddl/parser/domain.py index 0ce83071..4e13e30a 100644 --- a/pddl/parser/domain.py +++ b/pddl/parser/domain.py @@ -17,6 +17,7 @@ from lark import Lark, ParseError, Transformer from pddl.core import Action, Domain, Requirements +from pddl.custom_types import name from pddl.exceptions import PDDLMissingRequirementError, PDDLParsingError from pddl.helpers.base import assert_, safe_index from pddl.logic.base import ( @@ -294,7 +295,7 @@ def atomic_formula_skeleton(self, args): variables = [Variable(name, tags) for name, tags in variable_data.items()] return Predicate(name, *variables) - def typed_list_name(self, args) -> Dict[str, Optional[str]]: + def typed_list_name(self, args) -> Dict[name, Optional[name]]: """Process the 'typed_list_name' rule.""" try: types_index = TypesIndex.parse_typed_list(args) diff --git a/pddl/parser/types_index.py b/pddl/parser/types_index.py index 54ae6f40..42ed0efc 100644 --- a/pddl/parser/types_index.py +++ b/pddl/parser/types_index.py @@ -15,8 +15,9 @@ from collections import OrderedDict from typing import Dict, List, Optional from typing import OrderedDict as OrderedDictType -from typing import Set, Union, cast +from typing import Set, Union +from pddl.custom_types import name from pddl.helpers.base import check, safe_index from pddl.parser.symbols import Symbols @@ -33,39 +34,28 @@ class TypesIndex: def __init__(self) -> None: """Initialize the types index.""" - self._types_to_items: OrderedDictType[str, Set[str]] = OrderedDict() - self._item_to_types: OrderedDictType[str, Set[str]] = OrderedDict() + self._types_to_items: OrderedDictType[name, Set[name]] = OrderedDict() + self._item_to_types: OrderedDictType[name, Set[name]] = OrderedDict() - def add_item(self, item_name: str, type_tags: Set[str]) -> None: - """Add an item.""" - if item_name in self._item_to_types: - types_list = sorted(map(str, self._item_to_types[item_name])) - types_list_str = f" with types {types_list}" if len(types_list) > 0 else "" - raise ValueError( - f"duplicate name '{item_name}' in typed list already present" - + types_list_str - ) + def add_item(self, item_name: name, type_tags: Set[name]) -> None: + """ + Add an item to the types index with the given type tags. - exisiting_tags = self._item_to_types.get(item_name, set()) - for type_tag in type_tags: - if type_tag in exisiting_tags: - raise ValueError( - f"duplicate type tag '{type_tag}' in typed list: type already specified for item {item_name}" - ) + Both the item name and the type tags are validated according to the name type regular expression. - for type_tag in type_tags: - self._types_to_items.setdefault(type_tag, set()).add(item_name) - self._item_to_types.setdefault(item_name, set()).update(type_tags) + :param item_name: the item name + :param type_tags: the types for the item + """ + self._check_item_name_already_present(item_name) + self._check_tags_already_present(item_name, type_tags) + self._add_item(item_name, type_tags) - def get_typed_list_of_names(self) -> Dict[str, Optional[str]]: + def get_typed_list_of_names(self) -> Dict[name, Optional[name]]: """Get the typed list of names in form of dictionary.""" - result: Dict[str, Optional[str]] = {} + result: Dict[name, Optional[name]] = {} for item, types_tags in self._item_to_types.items(): if len(types_tags) > 1: - raise ValueError( - f"typed list names should not have more than one type, got '{item}' with " - f"types {sorted(map(str,types_tags))}" - ) + self._raise_multiple_types_error(item, types_tags) type_tag = next(iter(types_tags)) if len(types_tags) == 1 else None result[item] = type_tag return result @@ -157,6 +147,56 @@ def _add_typed_lists( Side-effect on the 'result' dictionary. The start_index and end_index are needed to avoid useless sublist copies. """ - for name in itertools.islice(tokens, start_index, end_index): - check(isinstance(name, str), f"invalid item '{name}' in typed list") - result.add_item(cast(str, name), type_tags) + for item_name in itertools.islice(tokens, start_index, end_index): + check( + isinstance(item_name, str), f"invalid item '{item_name}' in typed list" + ) + # these lines implicitly perform name validation + item_name = name(item_name) + type_tags_names: Set[name] = set(map(name, type_tags)) + result.add_item(item_name, type_tags_names) + + def _check_item_name_already_present(self, item_name: name) -> None: + """ + Check if the item name is already present in the index. + + :param item_name: the item name + """ + if item_name in self._item_to_types: + types_list = sorted(map(str, self._item_to_types[item_name])) + types_list_str = f" with types {types_list}" if len(types_list) > 0 else "" + raise ValueError( + f"duplicate name '{item_name}' in typed list already present" + + types_list_str + ) + + def _check_tags_already_present( + self, item_name: name, type_tags: Set[name] + ) -> None: + """ + Check if the type tags are already present for the given item name. + + :param item_name: the item name + :param type_tags: the type tags + """ + exisiting_tags = self._item_to_types.get(item_name, set()) + for type_tag in type_tags: + if type_tag in exisiting_tags: + raise ValueError( + f"duplicate type tag '{type_tag}' in typed list: type already specified for item {item_name}" + ) + + def _add_item(self, item_name: name, type_tags: Set[name]) -> None: + """Add an item (no validation).""" + for type_tag in type_tags: + self._types_to_items.setdefault(type_tag, set()).add(item_name) + self._item_to_types.setdefault(item_name, set()).update(type_tags) + + def _raise_multiple_types_error( + self, item_name: name, types_tags: Set[name] + ) -> None: + """Raise an error if the item has multiple types.""" + raise ValueError( + f"typed list names should not have more than one type, got '{item_name}' with " + f"types {sorted(map(str, types_tags))}" + ) From 48ffa56d7e8e6b37e2973858e0907382f9a7dd42 Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 11:59:13 +0200 Subject: [PATCH 11/51] chore: sort Symbols in alphanumerical order The order is taken from pag. 168 of the PDDL textbook, where colon ':' is ignored in determining the order. --- pddl/parser/symbols.py | 46 +++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/pddl/parser/symbols.py b/pddl/parser/symbols.py index f5c2acc0..77ade5bc 100644 --- a/pddl/parser/symbols.py +++ b/pddl/parser/symbols.py @@ -21,36 +21,36 @@ class Symbols(Enum): """A set of symbols that can be used in PDDL.""" + ROUND_BRACKET_LEFT = "(" + ROUND_BRACKET_RIGHT = ")" + TYPE_SEP = "-" + EQUAL = "=" + ACTION = ":action" + AND = "and" + CONSTANTS = ":constants" DEFINE = "define" + DERIVED = ":derived" DOMAIN = "domain" - PROBLEM = "problem" - AND = "and" - OR = "or" - NOT = "not" - IMPLY = "imply" - ONEOF = "oneof" - FORALL = "forall" + DOMAIN_P = ":domain" + EFFECT = ":effect" + EITHER = "either" EXISTS = "exists" - WHEN = "when" + FORALL = "forall" + GOAL = ":goal" + IMPLY = "imply" + INIT = ":init" + NOT = "not" OBJECT = "object" - EITHER = "either" - DERIVED = ":derived" - DOMAIN_P = ":domain" OBJECTS = ":objects" - INIT = ":init" - GOAL = ":goal" - REQUIREMENTS = ":requirements" - CONSTANTS = ":constants" - TYPES = ":types" - PREDICATES = ":predicates" - ACTION = ":action" + ONEOF = "oneof" + OR = "or" PARAMETERS = ":parameters" PRECONDITION = ":precondition" - EFFECT = ":effect" - ROUND_BRACKET_LEFT = "(" - ROUND_BRACKET_RIGHT = ")" - TYPE_SEP = "-" - EQUAL = "=" + PREDICATES = ":predicates" + PROBLEM = "problem" + REQUIREMENTS = ":requirements" + TYPES = ":types" + WHEN = "when" ALL_SYMBOLS: Set[str] = {v.value for v in Symbols} From 976f481c9b1d7b8c3bf5a44e882b773273df3ca1 Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 12:27:59 +0200 Subject: [PATCH 12/51] fix: add keyword validation for typed list names --- pddl/_validation.py | 10 +++++++-- pddl/parser/types_index.py | 26 ++++++++++++++++++++++- tests/conftest.py | 19 +++++++++++++++++ tests/test_parser.py | 42 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 94 insertions(+), 3 deletions(-) diff --git a/pddl/_validation.py b/pddl/_validation.py index 24ee27f5..8dfda497 100644 --- a/pddl/_validation.py +++ b/pddl/_validation.py @@ -16,10 +16,10 @@ from pddl.custom_types import name, to_names # noqa: F401 from pddl.exceptions import PDDLValidationError -from pddl.helpers.base import find_cycle +from pddl.helpers.base import ensure_set, find_cycle from pddl.logic import Constant, Predicate from pddl.logic.terms import Term -from pddl.parser.symbols import Symbols +from pddl.parser.symbols import ALL_SYMBOLS, Symbols def _check_types_dictionary(type_dict: Dict[name, Optional[name]]) -> None: @@ -110,3 +110,9 @@ def _check_types_in_has_terms_objects( f"type {repr(type_tag)} of term {repr(term)} in atomic expression " f"{repr(has_terms)} is not in available types {all_types}" ) + + +def _is_a_keyword(word: str, ignore: Optional[Set[str]] = None) -> bool: + """Check that the word is not a keyword.""" + ignore_set = ensure_set(ignore) + return word not in ignore_set and word in ALL_SYMBOLS diff --git a/pddl/parser/types_index.py b/pddl/parser/types_index.py index 42ed0efc..3629cbe3 100644 --- a/pddl/parser/types_index.py +++ b/pddl/parser/types_index.py @@ -13,10 +13,11 @@ """Utility to handle typed lists.""" import itertools from collections import OrderedDict -from typing import Dict, List, Optional +from typing import Collection, Dict, List, Optional from typing import OrderedDict as OrderedDictType from typing import Set, Union +from pddl._validation import _is_a_keyword from pddl.custom_types import name from pddl.helpers.base import check, safe_index from pddl.parser.symbols import Symbols @@ -46,6 +47,8 @@ def add_item(self, item_name: name, type_tags: Set[name]) -> None: :param item_name: the item name :param type_tags: the types for the item """ + self._check_names_not_a_keyword({item_name}) + self._check_types_not_a_keyword(type_tags) self._check_item_name_already_present(item_name) self._check_tags_already_present(item_name, type_tags) self._add_item(item_name, type_tags) @@ -200,3 +203,24 @@ def _raise_multiple_types_error( f"typed list names should not have more than one type, got '{item_name}' with " f"types {sorted(map(str, types_tags))}" ) + + def _check_names_not_a_keyword(self, names: Collection[name]) -> None: + """Check that items are not keywords.""" + self._check_not_a_keyword(names, "name", ignore=set()) + + def _check_types_not_a_keyword(self, names: Collection[name]) -> None: + """ + Check that types are not keywords. + + We ignore 'object' as it is a proper type. + """ + self._check_not_a_keyword(names, "type", ignore={Symbols.OBJECT.value}) + + def _check_not_a_keyword( + self, names: Collection[name], item_type: str, ignore: Set[str] + ) -> None: + """Check that the item name is not a keyword.""" + for item_name in names: + # we ignore 'object' as it is a proper type + if _is_a_keyword(item_name, ignore=ignore): + raise ValueError(f"invalid {item_type} '{item_name}': it is a keyword") diff --git a/tests/conftest.py b/tests/conftest.py index 1d117f63..fb82d2cb 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -21,6 +21,7 @@ import pddl from pddl.parser.domain import DomainParser from pddl.parser.problem import ProblemParser +from pddl.parser.symbols import Symbols _current_filepath = inspect.getframeinfo(inspect.currentframe()).filename # type: ignore TEST_DIRECTORY = Path(_current_filepath).absolute().parent @@ -114,3 +115,21 @@ def markdown_parser(): ] ################################################# + + +# A set of symbols that can be matched as names but they are keywords +# this is a subset of all symbols +TEXT_SYMBOLS = { + Symbols.AND.value, + Symbols.DEFINE.value, + Symbols.DOMAIN.value, + Symbols.EITHER.value, + Symbols.EXISTS.value, + Symbols.FORALL.value, + Symbols.NOT.value, + Symbols.OBJECT.value, + Symbols.ONEOF.value, + Symbols.OR.value, + Symbols.PROBLEM.value, + Symbols.WHEN.value, +} diff --git a/tests/test_parser.py b/tests/test_parser.py index c5182600..86079eca 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -20,11 +20,13 @@ from pddl.core import Domain, Problem from pddl.parser.domain import DomainParser +from pddl.parser.symbols import Symbols from tests.conftest import ( BLOCKSWORLD_FILES, BLOCKSWORLD_FOND_FILES, DOMAIN_FILES, PROBLEM_FILES, + TEXT_SYMBOLS, TRIANGLE_FILES, ) @@ -198,3 +200,43 @@ def test_constants_repetition_in_typed_lists_not_allowed() -> None: "duplicate name 'c1' in typed list already present with types \\['t1'\\]", ): DomainParser()(domain_str) + + +@pytest.mark.parametrize("keyword", TEXT_SYMBOLS - {Symbols.OBJECT.value}) +def test_keyword_usage_not_allowed_as_name(keyword) -> None: + """Check keywords usage as names is detected and a parsing error is raised.""" + domain_str = dedent( + f""" + (define (domain test) + (:requirements :typing) + (:types {keyword}) + ) + """ + ) + + with pytest.raises( + lark.exceptions.VisitError, + match=f".*error while parsing tokens \\['{keyword}'\\]: " + f"invalid name '{keyword}': it is a keyword", + ): + DomainParser()(domain_str) + + +@pytest.mark.parametrize("keyword", TEXT_SYMBOLS - {Symbols.OBJECT.value}) +def test_keyword_usage_not_allowed_as_type(keyword) -> None: + """Check keywords usage as types is detected and a parsing error is raised.""" + domain_str = dedent( + f""" + (define (domain test) + (:requirements :typing) + (:types t1 - {keyword}) + ) + """ + ) + + with pytest.raises( + lark.exceptions.VisitError, + match=f".*error while parsing tokens \\['t1', '-', '{keyword}'\\]: " + f"invalid type '{keyword}': it is a keyword", + ): + DomainParser()(domain_str) From 889b3fce85cb39deeba20d533fbbc6e8b953b593 Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 14:30:31 +0200 Subject: [PATCH 13/51] fix: renaming variables to avoid shadowing from outer scope --- pddl/parser/domain.py | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/pddl/parser/domain.py b/pddl/parser/domain.py index 4e13e30a..531fa7b4 100644 --- a/pddl/parser/domain.py +++ b/pddl/parser/domain.py @@ -110,7 +110,7 @@ def predicates(self, args): def action_def(self, args): """Process the 'action_def' rule.""" - name = args[2] + action_name = args[2] variables = args[4] # process action body @@ -118,7 +118,7 @@ def action_def(self, args): action_body = { _children[i][1:]: _children[i + 1] for i in range(0, len(_children), 2) } - return Action(name, variables, **action_body) + return Action(action_name, variables, **action_body) def derived_predicates(self, args): """Process the 'derived_predicates' rule.""" @@ -129,7 +129,7 @@ def derived_predicates(self, args): def action_parameters(self, args): """Process the 'action_parameters' rule.""" self._current_parameters_by_name = { - name: Variable(name, tags) for name, tags in args[1].items() + var_name: Variable(var_name, tags) for var_name, tags in args[1].items() } return list(self._current_parameters_by_name.values()) @@ -193,7 +193,7 @@ def gd_quantifiers(self, args): & self._extended_requirements ): raise PDDLMissingRequirementError(req) - variables = [Variable(name, tags) for name, tags in args[3].items()] + variables = [Variable(var_name, tags) for var_name, tags in args[3].items()] condition = args[5] return cond_class(cond=condition, variables=variables) @@ -232,7 +232,7 @@ def c_effect(self, args): if len(args) == 1: return args[0] if args[1] == Symbols.FORALL.value: - variables = [Variable(name, tags) for name, tags in args[3].items()] + variables = [Variable(var_name, tags) for var_name, tags in args[3].items()] return Forall(effect=args[-2], variables=variables) if args[1] == Symbols.WHEN.value: return When(args[2], args[3]) @@ -276,9 +276,9 @@ def constant_or_variable(t): right = constant_or_variable(args[3]) return EqualTo(left, right) else: - name = args[1] + predicate_name = args[1] terms = list(map(constant_or_variable, args[2:-1])) - return Predicate(name, *terms) + return Predicate(predicate_name, *terms) def constant(self, args): """Process the 'constant' rule.""" @@ -290,10 +290,12 @@ def constant(self, args): def atomic_formula_skeleton(self, args): """Process the 'atomic_formula_skeleton' rule.""" - name = args[1] + predicate_name = args[1] variable_data: Dict[str, Set[str]] = args[2] - variables = [Variable(name, tags) for name, tags in variable_data.items()] - return Predicate(name, *variables) + variables = [ + Variable(var_name, tags) for var_name, tags in variable_data.items() + ] + return Predicate(predicate_name, *variables) def typed_list_name(self, args) -> Dict[name, Optional[name]]: """Process the 'typed_list_name' rule.""" From 73e4facdb15f75c8cba42d26470dcbbf28419249 Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 14:28:10 +0200 Subject: [PATCH 14/51] feat: use TypesIndex to parse and validate typed_list_variables --- pddl/parser/domain.lark | 2 +- pddl/parser/domain.py | 90 ++++++++++---------------------------- pddl/parser/types_index.py | 4 ++ 3 files changed, 27 insertions(+), 69 deletions(-) diff --git a/pddl/parser/domain.lark b/pddl/parser/domain.lark index 09fed456..d8633f81 100644 --- a/pddl/parser/domain.lark +++ b/pddl/parser/domain.lark @@ -50,7 +50,7 @@ atomic_formula_term: LPAR predicate term* RPAR constant: NAME typed_list_variable: variable* - | variable+ TYPE_SEP type_def (typed_list_variable) + | (variable+ TYPE_SEP type_def)+ variable* ?variable: "?" NAME typed_list_name: NAME* diff --git a/pddl/parser/domain.py b/pddl/parser/domain.py index 531fa7b4..0e7e0a89 100644 --- a/pddl/parser/domain.py +++ b/pddl/parser/domain.py @@ -12,14 +12,16 @@ """Implementation of the PDDL domain parser.""" import sys -from typing import AbstractSet, Dict, List, Mapping, Optional, Sequence, Set, Tuple +from typing import Dict, Optional +from typing import OrderedDict as OrderedDictType +from typing import Set from lark import Lark, ParseError, Transformer from pddl.core import Action, Domain, Requirements from pddl.custom_types import name from pddl.exceptions import PDDLMissingRequirementError, PDDLParsingError -from pddl.helpers.base import assert_, safe_index +from pddl.helpers.base import assert_ from pddl.logic.base import ( And, ExistsCondition, @@ -307,7 +309,7 @@ def typed_list_name(self, args) -> Dict[name, Optional[name]]: f"error while parsing tokens {list(map(str, args))}: {str(e)}" ) from None - def typed_list_variable(self, args) -> Dict[str, Set[str]]: + def typed_list_variable(self, args) -> OrderedDictType[name, Set[name]]: """ Process the 'typed_list_variable' rule. @@ -316,79 +318,31 @@ def typed_list_variable(self, args) -> Dict[str, Set[str]]: :param args: the argument of this grammar rule :return: a typed list (variable), i.e. a mapping from variables to the supported types """ - type_sep_index = safe_index(args, Symbols.TYPE_SEP.value) - if type_sep_index is None: - result = self._parse_simple_typed_list(args, check_for_duplicates=False) - return {var: set() for var in result} - - # if we are here, the matched pattern is: [name_1 ... name_n], "-", type_def, other_typed_list_dict # noqa - # make sure there are only two tokens after "-" - assert_(len(args[type_sep_index:]) == 3, "unexpected parser state") - - variables: Tuple[str, ...] = tuple(args[:type_sep_index]) - type_def: Set[str] = self._process_type_def(args[type_sep_index + 1]) - other_typed_list_dict: Mapping[str, Set[str]] = args[type_sep_index + 2] - new_typed_list_dict: Mapping[str, Set[str]] = {v: type_def for v in variables} - - # check type conflicts - self._check_duplicates(other_typed_list_dict.keys(), new_typed_list_dict.keys()) - - return {**new_typed_list_dict, **other_typed_list_dict} + try: + types_index = TypesIndex.parse_typed_list(args) + return types_index.get_typed_list_of_variables() + except ValueError as e: + raise PDDLParsingError( + f"error while parsing tokens {list(map(str, args))}: {str(e)}" + ) from None def type_def(self, args): """Parse the 'type_def' rule.""" - return args if len(args) == 1 else args[1:-1] + assert_(len(args) != 0, "unexpected parser state: empty type_def") - def _has_requirement(self, requirement: Requirements) -> bool: - """Check whether a requirement is satisfied by the current state of the domain parsing.""" - return requirement in self._extended_requirements - - def _check_duplicates( - self, - other_names: AbstractSet[str], - new_names: AbstractSet[str], - ) -> None: - names_intersection = new_names & other_names - if len(names_intersection) != 0: - names_list_as_strings = map(repr, map(str, names_intersection)) - names_list_str = ", ".join(sorted(names_list_as_strings)) - raise PDDLParsingError( - f"detected conflicting items in a typed list: items occurred twice: [{names_list_str}]" - ) - - def _parse_simple_typed_list( - self, args: Sequence[str], check_for_duplicates: bool = True - ) -> Dict[str, Optional[str]]: - """ - Parse a 'simple' typed list. - - In this simple case, there are no type specifications, i.e. just a list of items. - - If check_for_duplicates is True, a check for duplicates is performed. - """ - # check for duplicates - if check_for_duplicates and len(set(args)) != len(args): - # find duplicates - seen = set() - dupes = [str(x) for x in args if x in seen or seen.add(x)] # type: ignore - raise PDDLParsingError( - f"duplicate items {dupes} found in the typed list: {list(map(str, args))}'" - ) - - return {arg: None for arg in args} - - def _process_type_def(self, type_def: List[str]) -> Set[str]: - """Process a raw type_def and return a set of types.""" - assert_(len(type_def) != 0, "unexpected parser state: empty type_def") - - if len(type_def) == 1: + if len(args) == 1: # single-typed type-def, return - return set(type_def) + return args # if we are here, type_def is of the form (either t1 ... tn) - either_keyword, types = type_def[0], type_def[1:] + # ignore first and last tokens since they are brackets. + either_keyword, types = args[1], args[2:-1] assert_(str(either_keyword) == Symbols.EITHER.value) - return set(types) + return types + + def _has_requirement(self, requirement: Requirements) -> bool: + """Check whether a requirement is satisfied by the current state of the domain parsing.""" + return requirement in self._extended_requirements _domain_parser_lark = DOMAIN_GRAMMAR_FILE.read_text() diff --git a/pddl/parser/types_index.py b/pddl/parser/types_index.py index 3629cbe3..121eec0a 100644 --- a/pddl/parser/types_index.py +++ b/pddl/parser/types_index.py @@ -63,6 +63,10 @@ def get_typed_list_of_names(self) -> Dict[name, Optional[name]]: result[item] = type_tag return result + def get_typed_list_of_variables(self) -> OrderedDictType[name, Set[name]]: + """Get the typed list of variables in form of dictionary.""" + return self._item_to_types + @classmethod def parse_typed_list(cls, tokens: List[Union[str, List[str]]]) -> "TypesIndex": """ From d2dded9947ae6ea776ebfab466fdc8edcc4de506 Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 14:53:21 +0200 Subject: [PATCH 15/51] test: split domain parser tests from problem parser tests --- tests/test_parser/__init__.py | 13 +++++ .../test_domain.py} | 30 +----------- tests/test_parser/test_problem.py | 47 +++++++++++++++++++ 3 files changed, 61 insertions(+), 29 deletions(-) create mode 100644 tests/test_parser/__init__.py rename tests/{test_parser.py => test_parser/test_domain.py} (87%) create mode 100644 tests/test_parser/test_problem.py diff --git a/tests/test_parser/__init__.py b/tests/test_parser/__init__.py new file mode 100644 index 00000000..04efc7b0 --- /dev/null +++ b/tests/test_parser/__init__.py @@ -0,0 +1,13 @@ +# +# Copyright 2021-2023 WhiteMech +# +# ------------------------------ +# +# This file is part of pddl. +# +# Use of this source code is governed by an MIT-style +# license that can be found in the LICENSE file or at +# https://opensource.org/licenses/MIT. +# + +"""Test parsing functions.""" diff --git a/tests/test_parser.py b/tests/test_parser/test_domain.py similarity index 87% rename from tests/test_parser.py rename to tests/test_parser/test_domain.py index 86079eca..390b6e10 100644 --- a/tests/test_parser.py +++ b/tests/test_parser/test_domain.py @@ -18,14 +18,13 @@ import pytest from pytest import lazy_fixture # type:ignore # noqa -from pddl.core import Domain, Problem +from pddl.core import Domain from pddl.parser.domain import DomainParser from pddl.parser.symbols import Symbols from tests.conftest import ( BLOCKSWORLD_FILES, BLOCKSWORLD_FOND_FILES, DOMAIN_FILES, - PROBLEM_FILES, TEXT_SYMBOLS, TRIANGLE_FILES, ) @@ -37,12 +36,6 @@ def test_domain_parser(domain_parser, pddl_file: Path): domain_parser(pddl_file.read_text()) -@pytest.mark.parametrize("pddl_file", PROBLEM_FILES) -def test_problem_parser(problem_parser, pddl_file: Path): - """Test only that the problem parsing works for all the fixtures.""" - problem_parser(pddl_file.read_text()) - - @pytest.mark.parametrize( "pddl_file,expected_domain", [ @@ -68,27 +61,6 @@ def test_check_domain_parser_output(domain_parser, pddl_file: Path, expected_dom assert actual_domain == expected_domain -@pytest.mark.parametrize( - "pddl_file,expected_problem", - [ - ( - BLOCKSWORLD_FILES / "p01.pddl", - lazy_fixture("blocksworld_problem_01"), # type:ignore - ), - ( - BLOCKSWORLD_FOND_FILES / "p01.pddl", - lazy_fixture("blocksworld_fond_01"), # type:ignore - ), - ], -) -def test_check_problem_parser_output(problem_parser, pddl_file: Path, expected_problem): - """Test problem parsing.""" - actual_problem = problem_parser(pddl_file.read_text()) - - assert isinstance(actual_problem, Problem) - assert actual_problem == expected_problem - - def test_hierarchical_types() -> None: """Test correct parsing of hierarchical types (see https://github.com/AI-Planning/pddl/issues/70).""" domain_str = dedent( diff --git a/tests/test_parser/test_problem.py b/tests/test_parser/test_problem.py new file mode 100644 index 00000000..129c23ee --- /dev/null +++ b/tests/test_parser/test_problem.py @@ -0,0 +1,47 @@ +# +# Copyright 2021-2023 WhiteMech +# +# ------------------------------ +# +# This file is part of pddl. +# +# Use of this source code is governed by an MIT-style +# license that can be found in the LICENSE file or at +# https://opensource.org/licenses/MIT. +# + +"""This module contains the tests for the domain parser.""" +from pathlib import Path + +import pytest +from pytest import lazy_fixture # type:ignore # noqa + +from pddl.core import Problem +from tests.conftest import BLOCKSWORLD_FILES, BLOCKSWORLD_FOND_FILES, PROBLEM_FILES + + +@pytest.mark.parametrize("pddl_file", PROBLEM_FILES) +def test_problem_parser(problem_parser, pddl_file: Path): + """Test only that the problem parsing works for all the fixtures.""" + problem_parser(pddl_file.read_text()) + + +@pytest.mark.parametrize( + "pddl_file,expected_problem", + [ + ( + BLOCKSWORLD_FILES / "p01.pddl", + lazy_fixture("blocksworld_problem_01"), # type:ignore + ), + ( + BLOCKSWORLD_FOND_FILES / "p01.pddl", + lazy_fixture("blocksworld_fond_01"), # type:ignore + ), + ], +) +def test_check_problem_parser_output(problem_parser, pddl_file: Path, expected_problem): + """Test problem parsing.""" + actual_problem = problem_parser(pddl_file.read_text()) + + assert isinstance(actual_problem, Problem) + assert actual_problem == expected_problem From 67d3b1d987a658b08962cd8cc0d541c90b869340 Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 14:55:28 +0200 Subject: [PATCH 16/51] docs: fix types argument passed to Domain in README The value of type dict passed to the Domain constructor in the README is set to None, denoting the fact that the type has no parent type. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 1beafc1e..8544953b 100644 --- a/README.md +++ b/README.md @@ -104,7 +104,7 @@ a1 = Action( requirements = [Requirements.STRIPS, Requirements.TYPING] domain = Domain("my_domain", requirements=requirements, - types={"type_1": []}, + types={"type_1": None}, constants=[a, b, c], predicates=[p1, p2], actions=[a1]) From 124a60a2050fdeb2342d084429641237c9683e52 Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 15:29:55 +0200 Subject: [PATCH 17/51] fix: problem optionally accepts requirements --- pddl/parser/problem.lark | 7 +++++-- pddl/parser/problem.py | 2 +- tests/test_parser/test_problem.py | 21 ++++++++++++++++++++- 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/pddl/parser/problem.lark b/pddl/parser/problem.lark index 9040fb77..58aee501 100644 --- a/pddl/parser/problem.lark +++ b/pddl/parser/problem.lark @@ -1,9 +1,12 @@ start: problem -problem: LPAR DEFINE problem_def problem_domain [requirements] [objects] init goal RPAR +problem: LPAR DEFINE problem_def problem_domain [problem_requirements] [objects] init goal RPAR problem_def: LPAR PROBLEM NAME RPAR problem_domain: LPAR DOMAIN_P NAME RPAR +problem_requirements: LPAR REQUIREMENTS require_key+ RPAR + + objects: LPAR OBJECTS typed_list_name RPAR init: LPAR INIT init_el* RPAR @@ -29,7 +32,7 @@ GOAL: ":goal" %ignore COMMENT %import .common.COMMENT -> COMMENT -%import .domain.requirements -> requirements +%import .domain.require_key -> require_key %import .domain.typed_list_name -> typed_list_name %import .domain.predicate -> predicate %import .common.NAME -> NAME diff --git a/pddl/parser/problem.py b/pddl/parser/problem.py index 7d47c6a9..33d2ee4b 100644 --- a/pddl/parser/problem.py +++ b/pddl/parser/problem.py @@ -57,7 +57,7 @@ def problem_domain(self, args): """Process the 'problem_domain' rule.""" return "domain_name", args[2] - def requirements(self, args): + def problem_requirements(self, args): """Process the 'requirements' rule.""" return "requirements", {Requirements(r[1:]) for r in args[2:-1]} diff --git a/tests/test_parser/test_problem.py b/tests/test_parser/test_problem.py index 129c23ee..d5e4ee06 100644 --- a/tests/test_parser/test_problem.py +++ b/tests/test_parser/test_problem.py @@ -12,11 +12,13 @@ """This module contains the tests for the domain parser.""" from pathlib import Path +from textwrap import dedent import pytest from pytest import lazy_fixture # type:ignore # noqa -from pddl.core import Problem +from pddl.core import Problem, Requirements +from pddl.parser.problem import ProblemParser from tests.conftest import BLOCKSWORLD_FILES, BLOCKSWORLD_FOND_FILES, PROBLEM_FILES @@ -45,3 +47,20 @@ def test_check_problem_parser_output(problem_parser, pddl_file: Path, expected_p assert isinstance(actual_problem, Problem) assert actual_problem == expected_problem + + +def test_problem_requirements_section_parsed() -> None: + """Check that the requirements section is parsed correctly.""" + problem_str = dedent( + """ + (define (problem test-problem) + (:domain test-domain) + (:requirements :typing) + (:objects a b c) + (:init (p b b b)) + (:goal (g a a a)) + )""" + ) + problem = ProblemParser()(problem_str) + + assert problem.requirements == {Requirements.TYPING} From 03abf7d4cd80609ad581739c98000d428517433b Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 15:36:52 +0200 Subject: [PATCH 18/51] lint: fix vulture's whitelist --- scripts/whitelist.py | 104 ++++++++++++++++++++----------------------- 1 file changed, 49 insertions(+), 55 deletions(-) diff --git a/scripts/whitelist.py b/scripts/whitelist.py index 89da7ad6..746d7e36 100644 --- a/scripts/whitelist.py +++ b/scripts/whitelist.py @@ -1,56 +1,50 @@ -# flake8: noqa -# type: ignore -# pylint: skip-file -find # unused function (pddl/helpers/base.py:86) -ensure_formula # unused function (pddl/logic/base.py:222) -PEffect # unused variable (pddl/logic/effects.py:180) -Effect # unused variable (pddl/logic/effects.py:182) -CondEffect # unused variable (pddl/logic/effects.py:183) -_._predicates_by_name # unused attribute (pddl/parser/domain.py:47) -_.start # unused method (pddl/parser/domain.py:52) -_.domain_def # unused method (pddl/parser/domain.py:72) -_._predicates_by_name # unused attribute (pddl/parser/domain.py:102) -_.action_def # unused method (pddl/parser/domain.py:105) -_.action_parameters # unused method (pddl/parser/domain.py:123) -_.emptyor_pregd # unused method (pddl/parser/domain.py:130) -_.gd # unused method (pddl/parser/domain.py:138) -_.emptyor_effect # unused method (pddl/parser/domain.py:168) -_.c_effect # unused method (pddl/parser/domain.py:187) -_.p_effect # unused method (pddl/parser/domain.py:197) -_.cond_effect # unused method (pddl/parser/domain.py:204) -_.atomic_formula_term # unused method (pddl/parser/domain.py:212) -_.atomic_formula_skeleton # unused method (pddl/parser/domain.py:237) -_.typed_list_name # unused method (pddl/parser/domain.py:244) -_.typed_list_variable # unused method (pddl/parser/domain.py:255) -_.type_def # unused method (pddl/parser/domain.py:282) -_.start # unused method (pddl/parser/problem.py:47) -_.problem_def # unused method (pddl/parser/problem.py:55) -_.problem_domain # unused method (pddl/parser/problem.py:59) -_.typed_list_name # unused method (pddl/parser/problem.py:76) -_.domain__type_def # unused method (pddl/parser/problem.py:87) -_.literal_name # unused method (pddl/parser/problem.py:96) -_.gd_name # unused method (pddl/parser/problem.py:109) -_.atomic_formula_name # unused method (pddl/parser/problem.py:120) -OpSymbol # unused variable (pddl/parser/symbols.py:27) -OpRequirement # unused variable (pddl/parser/symbols.py:28) -DEFINE # unused variable (pddl/parser/symbols.py:34) -DOMAIN # unused variable (pddl/parser/symbols.py:35) -PROBLEM # unused variable (pddl/parser/symbols.py:36) -EXISTS # unused variable (pddl/parser/symbols.py:43) -DOMAIN_P # unused variable (pddl/parser/symbols.py:46) -OBJECTS # unused variable (pddl/parser/symbols.py:47) -INIT # unused variable (pddl/parser/symbols.py:48) -GOAL # unused variable (pddl/parser/symbols.py:49) -REQUIREMENTS # unused variable (pddl/parser/symbols.py:50) -CONSTANTS # unused variable (pddl/parser/symbols.py:51) +safe_get # unused function (pddl/helpers/base.py:95) +find # unused function (pddl/helpers/base.py:100) +ensure_formula # unused function (pddl/logic/base.py:294) +PEffect # unused variable (pddl/logic/effects.py:168) +Effect # unused variable (pddl/logic/effects.py:170) +CondEffect # unused variable (pddl/logic/effects.py:171) +_._predicates_by_name # unused attribute (pddl/parser/domain.py:51) +_.start # unused method (pddl/parser/domain.py:56) +_.domain_def # unused method (pddl/parser/domain.py:77) +_._predicates_by_name # unused attribute (pddl/parser/domain.py:110) +_.action_def # unused method (pddl/parser/domain.py:113) +_.action_parameters # unused method (pddl/parser/domain.py:131) +_.emptyor_pregd # unused method (pddl/parser/domain.py:138) +_.gd # unused method (pddl/parser/domain.py:202) +_.emptyor_effect # unused method (pddl/parser/domain.py:217) +_.c_effect # unused method (pddl/parser/domain.py:232) +_.p_effect # unused method (pddl/parser/domain.py:247) +_.cond_effect # unused method (pddl/parser/domain.py:254) +_.atomic_formula_term # unused method (pddl/parser/domain.py:262) +_.atomic_formula_skeleton # unused method (pddl/parser/domain.py:293) +_.typed_list_variable # unused method (pddl/parser/domain.py:312) +_.type_def # unused method (pddl/parser/domain.py:329) +_.start # unused method (pddl/parser/problem.py:39) +_.problem_def # unused method (pddl/parser/problem.py:52) +_.problem_domain # unused method (pddl/parser/problem.py:56) +_.problem_requirements # unused method (pddl/parser/problem.py:60) +_.domain__type_def # unused method (pddl/parser/problem.py:83) +_.literal_name # unused method (pddl/parser/problem.py:92) +_.gd_name # unused method (pddl/parser/problem.py:105) +_.atomic_formula_name # unused method (pddl/parser/problem.py:116) +OpSymbol # unused variable (pddl/parser/symbols.py:17) +OpRequirement # unused variable (pddl/parser/symbols.py:18) +ROUND_BRACKET_LEFT # unused variable (pddl/parser/symbols.py:24) +ROUND_BRACKET_RIGHT # unused variable (pddl/parser/symbols.py:25) +ACTION # unused variable (pddl/parser/symbols.py:28) +CONSTANTS # unused variable (pddl/parser/symbols.py:30) +DEFINE # unused variable (pddl/parser/symbols.py:31) +DOMAIN # unused variable (pddl/parser/symbols.py:33) +DOMAIN_P # unused variable (pddl/parser/symbols.py:34) +EFFECT # unused variable (pddl/parser/symbols.py:35) +GOAL # unused variable (pddl/parser/symbols.py:39) +INIT # unused variable (pddl/parser/symbols.py:41) +OBJECTS # unused variable (pddl/parser/symbols.py:44) +PARAMETERS # unused variable (pddl/parser/symbols.py:47) +PRECONDITION # unused variable (pddl/parser/symbols.py:48) +PREDICATES # unused variable (pddl/parser/symbols.py:49) +PROBLEM # unused variable (pddl/parser/symbols.py:50) +REQUIREMENTS # unused variable (pddl/parser/symbols.py:51) TYPES # unused variable (pddl/parser/symbols.py:52) -PREDICATES # unused variable (pddl/parser/symbols.py:53) -ACTION # unused variable (pddl/parser/symbols.py:54) -PARAMETERS # unused variable (pddl/parser/symbols.py:55) -PRECONDITION # unused variable (pddl/parser/symbols.py:56) -EFFECT # unused variable (pddl/parser/symbols.py:57) -ROUND_BRACKET_LEFT # unused variable (pddl/parser/symbols.py:58) -ROUND_BRACKET_RIGHT # unused variable (pddl/parser/symbols.py:59) -ALL_SYMBOLS # unused variable (pddl/parser/symbols.py:64) -ALL_REQUIREMENTS # unused variable (pddl/parser/symbols.py:85) -safe_get # unused function (pddl/helpers/base.py:88) +ALL_REQUIREMENTS # unused variable (pddl/parser/symbols.py:80) From 04fe2f8b312d06af276393aed9ede80adafb3d31 Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 16:36:28 +0200 Subject: [PATCH 19/51] feat: add internal class to manage and handle types --- pddl/_validation.py | 51 ++--------------- pddl/core.py | 129 +++++++++++++++++++++++++++++++++++-------- tests/test_domain.py | 10 +++- 3 files changed, 117 insertions(+), 73 deletions(-) diff --git a/pddl/_validation.py b/pddl/_validation.py index 8dfda497..dacf7709 100644 --- a/pddl/_validation.py +++ b/pddl/_validation.py @@ -11,57 +11,14 @@ # """This module defines validation functions for PDDL data structures.""" +from typing import Collection, Optional, Set, Tuple -from typing import Collection, Dict, Optional, Set, Tuple - -from pddl.custom_types import name, to_names # noqa: F401 +from pddl.custom_types import name, namelike, to_names # noqa: F401 from pddl.exceptions import PDDLValidationError -from pddl.helpers.base import ensure_set, find_cycle +from pddl.helpers.base import ensure_set from pddl.logic import Constant, Predicate from pddl.logic.terms import Term -from pddl.parser.symbols import ALL_SYMBOLS, Symbols - - -def _check_types_dictionary(type_dict: Dict[name, Optional[name]]) -> None: - """ - Check the consistency of the types dictionary. - - 1) Empty types dictionary is correct by definition: - >>> _check_types_dictionary({}) - - 2) The `object` type cannot be a subtype: - >>> a = name("a") - >>> _check_types_dictionary({name("object"): a}) - Traceback (most recent call last): - ... - pddl.exceptions.PDDLValidationError: object must not have supertypes, but got 'object' is a subtype of 'a' - - 3) If cycles in the type hierarchy graph are present, an error is raised: - >>> a, b, c = to_names(["a", "b", "c"]) - >>> _check_types_dictionary({a: b, b: c, c: a}) - Traceback (most recent call last): - ... - pddl.exceptions.PDDLValidationError: cycle detected in the type hierarchy: a -> b -> c - - :param type_dict: the types dictionary - """ - if len(type_dict) == 0: - return - - # check `object` type - object_name = name(Symbols.OBJECT.value) - if object_name in type_dict and type_dict[object_name] is not None: - object_supertype = type_dict[object_name] - raise PDDLValidationError( - f"object must not have supertypes, but got 'object' is a subtype of '{object_supertype}'" - ) - - # check cycles - cycle = find_cycle(type_dict) # type: ignore - if cycle is not None: - raise PDDLValidationError( - "cycle detected in the type hierarchy: " + " -> ".join(cycle) - ) +from pddl.parser.symbols import ALL_SYMBOLS def _find_inconsistencies_in_typed_terms( diff --git a/pddl/core.py b/pddl/core.py index 9d0e8cf2..347b5db2 100644 --- a/pddl/core.py +++ b/pddl/core.py @@ -19,24 +19,23 @@ from enum import Enum from typing import AbstractSet, Collection, Dict, Optional, Sequence, Set, cast -from pddl._validation import ( - _check_constant_types, - _check_types_dictionary, - _check_types_in_has_terms_objects, -) +from pddl._validation import _check_constant_types, _check_types_in_has_terms_objects from pddl.custom_types import name as name_type -from pddl.custom_types import namelike, to_names_types +from pddl.custom_types import namelike, to_names, to_names_types # noqa: F401 +from pddl.exceptions import PDDLValidationError from pddl.helpers.base import ( _typed_parameters, assert_, ensure, ensure_sequence, ensure_set, + find_cycle, ) from pddl.logic.base import Formula, TrueFormula, is_literal from pddl.logic.predicates import DerivedPredicate, Predicate from pddl.logic.terms import Constant, Term, Variable from pddl.parser.symbols import RequirementSymbols as RS +from pddl.parser.symbols import Symbols class Domain: @@ -68,30 +67,19 @@ def __init__( """ self._name = name_type(name) self._requirements = ensure_set(requirements) - self._types = to_names_types(ensure(types, dict())) + self._types = _Types(types, self._requirements) self._constants = ensure_set(constants) self._predicates = ensure_set(predicates) self._derived_predicates = ensure_set(derived_predicates) self._actions = ensure_set(actions) - self._all_types_set = self._get_all_types() - self._check_consistency() - def _get_all_types(self) -> Set[name_type]: - """Get all types supported by this domain.""" - if self._types is None: - return set() - result = set(self._types.keys()) | set(self._types.values()) - result.discard(None) - return cast(Set[name_type], result) - def _check_consistency(self) -> None: """Check consistency of a domain instance object.""" - _check_types_dictionary(self._types) - _check_constant_types(self._constants, self._all_types_set) - _check_types_in_has_terms_objects(self._predicates, self._all_types_set) - _check_types_in_has_terms_objects(self._actions, self._all_types_set) # type: ignore + _check_constant_types(self._constants, self._types.all_types) + _check_types_in_has_terms_objects(self._predicates, self._types.all_types) + _check_types_in_has_terms_objects(self._actions, self._types.all_types) # type: ignore self._check_types_in_derived_predicates() def _check_types_in_derived_predicates(self) -> None: @@ -101,7 +89,7 @@ def _check_types_in_derived_predicates(self) -> None: if self._derived_predicates else set() ) - _check_types_in_has_terms_objects(dp_list, self._all_types_set) + _check_types_in_has_terms_objects(dp_list, self._types.all_types) @property def name(self) -> str: @@ -136,7 +124,7 @@ def actions(self) -> AbstractSet["Action"]: @property def types(self) -> Dict[name_type, Optional[name_type]]: """Get the type definitions, if defined. Else, raise error.""" - return self._types + return self._types.raw def __eq__(self, other): """Compare with another object.""" @@ -388,3 +376,98 @@ def __lt__(self, other): return self.value <= other.value else: return super().__lt__(other) + + +class _Types: + """A class for representing and managing the types available in a PDDL Domain.""" + + def __init__( + self, + types: Optional[Dict[namelike, Optional[namelike]]] = None, + requirements: Optional[AbstractSet[Requirements]] = None, + ) -> None: + """Initialize the Types object.""" + self._types = to_names_types(ensure(types, dict())) + + self._all_types = self._get_all_types() + self._check_types_dictionary(self._types, ensure_set(requirements)) + + @property + def raw(self) -> Dict[name_type, Optional[name_type]]: + """Get the raw types dictionary.""" + return self._types + + @property + def all_types(self) -> Set[name_type]: + """Get all available types.""" + return self._all_types + + def _get_all_types(self) -> Set[name_type]: + """Get all types supported by the domain.""" + if self._types is None: + return set() + result = set(self._types.keys()) | set(self._types.values()) + result.discard(None) + return cast(Set[name_type], result) + + @classmethod + def _check_types_dictionary( + cls, + type_dict: Dict[name_type, Optional[name_type]], + requirements: AbstractSet[Requirements], + ) -> None: + """ + Check the consistency of the types dictionary. + + 1) Empty types dictionary is correct by definition: + >>> _Types._check_types_dictionary({}, set()) + + 2) There are supertypes, but :typing requirement not specified + >>> a, b, c = to_names(["a", "b", "c"]) + >>> _Types._check_types_dictionary({a: b, b: c}, set()) + Traceback (most recent call last): + ... + pddl.exceptions.PDDLValidationError: typing requirement is not specified, but types are used: 'b', 'c' + + 3) The `object` type cannot be a subtype: + >>> a = name_type("a") + >>> _Types._check_types_dictionary({name_type("object"): a}, {Requirements.TYPING}) + Traceback (most recent call last): + ... + pddl.exceptions.PDDLValidationError: object must not have supertypes, but got 'object' is a subtype of 'a' + + 4) If cycles in the type hierarchy graph are present, an error is raised: + >>> a, b, c = to_names(["a", "b", "c"]) + >>> _Types._check_types_dictionary({a: b, b: c, c: a}, {Requirements.TYPING}) + Traceback (most recent call last): + ... + pddl.exceptions.PDDLValidationError: cycle detected in the type hierarchy: a -> b -> c + + :param type_dict: the types dictionary + """ + if len(type_dict) == 0: + return + + # check typing requirement + supertypes = {t for t in type_dict.values() if t is not None} + if len(supertypes) > 0 and Requirements.TYPING not in requirements: + raise PDDLValidationError( + "typing requirement is not specified, but types are used: '" + + "', '".join(map(str, sorted(supertypes))) + + "'" + ) + + # check `object` type + object_name = name_type(Symbols.OBJECT.value) + if object_name in type_dict and type_dict[object_name] is not None: + object_supertype = type_dict[object_name] + raise PDDLValidationError( + f"object must not have supertypes, but got 'object' is a subtype of '{object_supertype}'" + ) + + # check cycles + cycle = find_cycle(type_dict) # type: ignore + if cycle is not None: + raise PDDLValidationError( + "cycle detected in the type hierarchy: " + " -> ".join(cycle) + ) diff --git a/tests/test_domain.py b/tests/test_domain.py index d8d35e92..a3668538 100644 --- a/tests/test_domain.py +++ b/tests/test_domain.py @@ -17,7 +17,7 @@ import pytest -from pddl.core import Action, Domain +from pddl.core import Action, Domain, Requirements from pddl.exceptions import PDDLValidationError from pddl.logic import Constant, Variable from pddl.logic.base import Not, TrueFormula @@ -88,7 +88,11 @@ def test_cycles_in_type_defs_not_allowed() -> None: with pytest.raises( PDDLValidationError, match="cycle detected in the type hierarchy: A -> B -> C" ): - Domain("dummy", types={"A": "B", "B": "C", "C": "A"}) + Domain( + "dummy", + requirements={Requirements.TYPING}, + types={"A": "B", "B": "C", "C": "A"}, + ) def test_object_must_not_be_subtype() -> None: @@ -100,7 +104,7 @@ def test_object_must_not_be_subtype() -> None: PDDLValidationError, match=rf"object must not have supertypes, but got 'object' is a subtype of '{my_type}'", ): - Domain("test", types=type_set) # type: ignore + Domain("test", requirements={Requirements.TYPING}, types=type_set) # type: ignore def test_constants_type_not_available() -> None: From 1a35939ea01724f7fd28109e244958125319cefc Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 17:16:46 +0200 Subject: [PATCH 20/51] refactor: more Requirements to its own module pddl.requirements This is needed to accomodate future changes and to avoid circular imports. --- README.md | 14 +++-- pddl/core.py | 48 +------------- pddl/parser/domain.py | 3 +- pddl/parser/problem.py | 3 +- pddl/requirements.py | 62 +++++++++++++++++++ .../fixtures/code_objects/blocksworld_fond.py | 3 +- .../code_objects/blocksworld_ipc08.py | 3 +- .../code_objects/triangle_tireworld.py | 3 +- tests/test_domain.py | 3 +- tests/test_parser/test_problem.py | 3 +- 10 files changed, 85 insertions(+), 60 deletions(-) create mode 100644 pddl/requirements.py diff --git a/README.md b/README.md index 8544953b..1f1a87d7 100644 --- a/README.md +++ b/README.md @@ -79,10 +79,12 @@ You can use the `pddl` package in two ways: as a library, and as a CLI tool. This is an example of how you can build a PDDL domain or problem programmatically: + ```python from pddl.logic import Predicate, constants, variables -from pddl.core import Domain, Problem, Action, Requirements +from pddl.core import Domain, Problem, Action from pddl.formatter import domain_to_string, problem_to_string +from pddl.requirements import Requirements # set up variables and constants x, y, z = variables("x y z", types=["type_1"]) @@ -103,11 +105,11 @@ a1 = Action( # define the domain object. requirements = [Requirements.STRIPS, Requirements.TYPING] domain = Domain("my_domain", - requirements=requirements, - types={"type_1": None}, - constants=[a, b, c], - predicates=[p1, p2], - actions=[a1]) + requirements=requirements, + types={"type_1": None}, + constants=[a, b, c], + predicates=[p1, p2], + actions=[a1]) print(domain_to_string(domain)) ``` diff --git a/pddl/core.py b/pddl/core.py index 347b5db2..d9a1aff4 100644 --- a/pddl/core.py +++ b/pddl/core.py @@ -15,8 +15,6 @@ It contains the class definitions to build and modify PDDL domains or problems. """ -import functools -from enum import Enum from typing import AbstractSet, Collection, Dict, Optional, Sequence, Set, cast from pddl._validation import _check_constant_types, _check_types_in_has_terms_objects @@ -34,8 +32,8 @@ from pddl.logic.base import Formula, TrueFormula, is_literal from pddl.logic.predicates import DerivedPredicate, Predicate from pddl.logic.terms import Constant, Term, Variable -from pddl.parser.symbols import RequirementSymbols as RS from pddl.parser.symbols import Symbols +from pddl.requirements import Requirements class Domain: @@ -334,50 +332,6 @@ def __repr__(self) -> str: ) -@functools.total_ordering -class Requirements(Enum): - """Enum class for the requirements.""" - - STRIPS = RS.STRIPS.strip() - TYPING = RS.TYPING.strip() - NEG_PRECONDITION = RS.NEG_PRECONDITION.strip() - DIS_PRECONDITION = RS.DIS_PRECONDITION.strip() - UNIVERSAL_PRECONDITION = RS.UNIVERSAL_PRECONDITION.strip() - EXISTENTIAL_PRECONDITION = RS.EXISTENTIAL_PRECONDITION.strip() - QUANTIFIED_PRECONDITION = RS.QUANTIFIED_PRECONDITION.strip() - EQUALITY = RS.EQUALITY.strip() - CONDITIONAL_EFFECTS = RS.CONDITIONAL_EFFECTS.strip() - ADL = RS.ADL.strip() - DERIVED_PREDICATES = RS.DERIVED_PREDICATES.strip() - NON_DETERMINISTIC = RS.NON_DETERMINISTIC.strip() - - @classmethod - def strips_requirements(cls) -> Set["Requirements"]: - """Get the STRIPS requirements.""" - return { - Requirements.TYPING, - Requirements.NEG_PRECONDITION, - Requirements.DIS_PRECONDITION, - Requirements.EQUALITY, - Requirements.CONDITIONAL_EFFECTS, - } - - def __str__(self) -> str: - """Get the string representation.""" - return f":{self.value}" - - def __repr__(self) -> str: - """Get an unambiguous representation.""" - return f"Requirements{self.name}" - - def __lt__(self, other): - """Compare with another object.""" - if isinstance(other, Requirements): - return self.value <= other.value - else: - return super().__lt__(other) - - class _Types: """A class for representing and managing the types available in a PDDL Domain.""" diff --git a/pddl/parser/domain.py b/pddl/parser/domain.py index 0e7e0a89..b02d6821 100644 --- a/pddl/parser/domain.py +++ b/pddl/parser/domain.py @@ -18,7 +18,7 @@ from lark import Lark, ParseError, Transformer -from pddl.core import Action, Domain, Requirements +from pddl.core import Action, Domain from pddl.custom_types import name from pddl.exceptions import PDDLMissingRequirementError, PDDLParsingError from pddl.helpers.base import assert_ @@ -38,6 +38,7 @@ from pddl.parser import DOMAIN_GRAMMAR_FILE, PARSERS_DIRECTORY from pddl.parser.symbols import Symbols from pddl.parser.types_index import TypesIndex +from pddl.requirements import Requirements class DomainTransformer(Transformer): diff --git a/pddl/parser/problem.py b/pddl/parser/problem.py index 33d2ee4b..4d5b56af 100644 --- a/pddl/parser/problem.py +++ b/pddl/parser/problem.py @@ -16,7 +16,7 @@ from lark import Lark, ParseError, Transformer -from pddl.core import Problem, Requirements +from pddl.core import Problem from pddl.helpers.base import assert_ from pddl.logic.base import And, Not from pddl.logic.predicates import EqualTo, Predicate @@ -24,6 +24,7 @@ from pddl.parser import PARSERS_DIRECTORY, PROBLEM_GRAMMAR_FILE from pddl.parser.domain import DomainTransformer from pddl.parser.symbols import Symbols +from pddl.requirements import Requirements class ProblemTransformer(Transformer): diff --git a/pddl/requirements.py b/pddl/requirements.py new file mode 100644 index 00000000..1d67582d --- /dev/null +++ b/pddl/requirements.py @@ -0,0 +1,62 @@ +# +# Copyright 2021-2023 WhiteMech +# +# ------------------------------ +# +# This file is part of pddl. +# +# Use of this source code is governed by an MIT-style +# license that can be found in the LICENSE file or at +# https://opensource.org/licenses/MIT. +# + +"""This module contains the definition of the PDDL requirements.""" +import functools +from enum import Enum +from typing import Set + +from pddl.parser.symbols import RequirementSymbols as RS + + +@functools.total_ordering +class Requirements(Enum): + """Enum class for the requirements.""" + + STRIPS = RS.STRIPS.strip() + TYPING = RS.TYPING.strip() + NEG_PRECONDITION = RS.NEG_PRECONDITION.strip() + DIS_PRECONDITION = RS.DIS_PRECONDITION.strip() + UNIVERSAL_PRECONDITION = RS.UNIVERSAL_PRECONDITION.strip() + EXISTENTIAL_PRECONDITION = RS.EXISTENTIAL_PRECONDITION.strip() + QUANTIFIED_PRECONDITION = RS.QUANTIFIED_PRECONDITION.strip() + EQUALITY = RS.EQUALITY.strip() + CONDITIONAL_EFFECTS = RS.CONDITIONAL_EFFECTS.strip() + ADL = RS.ADL.strip() + DERIVED_PREDICATES = RS.DERIVED_PREDICATES.strip() + NON_DETERMINISTIC = RS.NON_DETERMINISTIC.strip() + + @classmethod + def strips_requirements(cls) -> Set["Requirements"]: + """Get the STRIPS requirements.""" + return { + Requirements.TYPING, + Requirements.NEG_PRECONDITION, + Requirements.DIS_PRECONDITION, + Requirements.EQUALITY, + Requirements.CONDITIONAL_EFFECTS, + } + + def __str__(self) -> str: + """Get the string representation.""" + return f":{self.value}" + + def __repr__(self) -> str: + """Get an unambiguous representation.""" + return f"Requirements{self.name}" + + def __lt__(self, other): + """Compare with another object.""" + if isinstance(other, Requirements): + return self.value <= other.value + else: + return super().__lt__(other) diff --git a/tests/fixtures/code_objects/blocksworld_fond.py b/tests/fixtures/code_objects/blocksworld_fond.py index 06683ab4..216d1cb2 100644 --- a/tests/fixtures/code_objects/blocksworld_fond.py +++ b/tests/fixtures/code_objects/blocksworld_fond.py @@ -13,12 +13,13 @@ """This test module contains the fixtures for 'blocksworld-ipc08' domain and problem.""" import pytest -from pddl.core import Action, Domain, Problem, Requirements +from pddl.core import Action, Domain, Problem from pddl.logic import Constant from pddl.logic.base import And, OneOf from pddl.logic.effects import AndEffect, When from pddl.logic.helpers import constants, variables from pddl.logic.predicates import EqualTo, Predicate +from pddl.requirements import Requirements @pytest.fixture(scope="session") diff --git a/tests/fixtures/code_objects/blocksworld_ipc08.py b/tests/fixtures/code_objects/blocksworld_ipc08.py index d1932a4c..efd4c4fa 100644 --- a/tests/fixtures/code_objects/blocksworld_ipc08.py +++ b/tests/fixtures/code_objects/blocksworld_ipc08.py @@ -13,11 +13,12 @@ """This test module contains the fixtures for 'blocksworld-ipc08' domain and problem.""" import pytest -from pddl.core import Action, Domain, Problem, Requirements +from pddl.core import Action, Domain, Problem from pddl.logic.base import And, OneOf from pddl.logic.effects import AndEffect from pddl.logic.helpers import constants, variables from pddl.logic.predicates import EqualTo, Predicate +from pddl.requirements import Requirements @pytest.fixture(scope="session") diff --git a/tests/fixtures/code_objects/triangle_tireworld.py b/tests/fixtures/code_objects/triangle_tireworld.py index c26587f6..e50b3ecc 100644 --- a/tests/fixtures/code_objects/triangle_tireworld.py +++ b/tests/fixtures/code_objects/triangle_tireworld.py @@ -13,11 +13,12 @@ """This test module contains the fixtures for 'triangle-tireworld' domain and problem.""" import pytest -from pddl.core import Action, Domain, Problem, Requirements +from pddl.core import Action, Domain, Problem from pddl.logic.base import And, OneOf from pddl.logic.effects import AndEffect from pddl.logic.helpers import constants, variables from pddl.logic.predicates import Predicate +from pddl.requirements import Requirements @pytest.fixture(scope="session") diff --git a/tests/test_domain.py b/tests/test_domain.py index a3668538..d90ccadd 100644 --- a/tests/test_domain.py +++ b/tests/test_domain.py @@ -17,13 +17,14 @@ import pytest -from pddl.core import Action, Domain, Requirements +from pddl.core import Action, Domain from pddl.exceptions import PDDLValidationError from pddl.logic import Constant, Variable from pddl.logic.base import Not, TrueFormula from pddl.logic.helpers import constants, variables from pddl.logic.predicates import DerivedPredicate, Predicate from pddl.parser.symbols import Symbols +from pddl.requirements import Requirements from tests.conftest import pddl_objects_domains diff --git a/tests/test_parser/test_problem.py b/tests/test_parser/test_problem.py index d5e4ee06..26467079 100644 --- a/tests/test_parser/test_problem.py +++ b/tests/test_parser/test_problem.py @@ -17,8 +17,9 @@ import pytest from pytest import lazy_fixture # type:ignore # noqa -from pddl.core import Problem, Requirements +from pddl.core import Problem from pddl.parser.problem import ProblemParser +from pddl.requirements import Requirements from tests.conftest import BLOCKSWORLD_FILES, BLOCKSWORLD_FOND_FILES, PROBLEM_FILES From c3ab1aa32f79ca35e7e4987204e099d09e1b76e0 Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 17:26:27 +0200 Subject: [PATCH 21/51] chore: move _Types in pddl._validation --- pddl/_validation.py | 113 ++++++++++++++++++++++++++++++++++++++++---- pddl/core.py | 108 +++--------------------------------------- 2 files changed, 112 insertions(+), 109 deletions(-) diff --git a/pddl/_validation.py b/pddl/_validation.py index dacf7709..78b3aa6e 100644 --- a/pddl/_validation.py +++ b/pddl/_validation.py @@ -11,19 +11,21 @@ # """This module defines validation functions for PDDL data structures.""" -from typing import Collection, Optional, Set, Tuple +from typing import AbstractSet, Collection, Dict, Optional, Set, Tuple, cast -from pddl.custom_types import name, namelike, to_names # noqa: F401 +from pddl.custom_types import name as name_type +from pddl.custom_types import namelike, to_names, to_names_types # noqa: F401 from pddl.exceptions import PDDLValidationError -from pddl.helpers.base import ensure_set +from pddl.helpers.base import ensure, ensure_set, find_cycle from pddl.logic import Constant, Predicate from pddl.logic.terms import Term -from pddl.parser.symbols import ALL_SYMBOLS +from pddl.parser.symbols import ALL_SYMBOLS, Symbols +from pddl.requirements import Requirements def _find_inconsistencies_in_typed_terms( - terms: Optional[Collection[Term]], all_types: Set[name] -) -> Optional[Tuple[Term, name]]: + terms: Optional[Collection[Term]], all_types: Set[name_type] +) -> Optional[Tuple[Term, name_type]]: """ Check that the terms in input all have legal types according to the list of available types. @@ -41,7 +43,7 @@ def _find_inconsistencies_in_typed_terms( def _check_constant_types( - constants: Optional[Collection[Constant]], all_types: Set[name] + constants: Optional[Collection[Constant]], all_types: Set[name_type] ) -> None: check_result = _find_inconsistencies_in_typed_terms(constants, all_types) if check_result is not None: @@ -53,7 +55,7 @@ def _check_constant_types( def _check_types_in_has_terms_objects( has_terms_objects: Optional[Collection[Predicate]], - all_types: Set[name], + all_types: Set[name_type], ) -> None: """Check that the terms in the set of predicates all have legal types.""" if has_terms_objects is None: @@ -73,3 +75,98 @@ def _is_a_keyword(word: str, ignore: Optional[Set[str]] = None) -> bool: """Check that the word is not a keyword.""" ignore_set = ensure_set(ignore) return word not in ignore_set and word in ALL_SYMBOLS + + +class Types: + """A class for representing and managing the types available in a PDDL Domain.""" + + def __init__( + self, + types: Optional[Dict[namelike, Optional[namelike]]] = None, + requirements: Optional[AbstractSet[Requirements]] = None, + ) -> None: + """Initialize the Types object.""" + self._types = to_names_types(ensure(types, dict())) + + self._all_types = self._get_all_types() + self._check_types_dictionary(self._types, ensure_set(requirements)) + + @property + def raw(self) -> Dict[name_type, Optional[name_type]]: + """Get the raw types dictionary.""" + return self._types + + @property + def all_types(self) -> Set[name_type]: + """Get all available types.""" + return self._all_types + + def _get_all_types(self) -> Set[name_type]: + """Get all types supported by the domain.""" + if self._types is None: + return set() + result = set(self._types.keys()) | set(self._types.values()) + result.discard(None) + return cast(Set[name_type], result) + + @classmethod + def _check_types_dictionary( + cls, + type_dict: Dict[name_type, Optional[name_type]], + requirements: AbstractSet[Requirements], + ) -> None: + """ + Check the consistency of the types dictionary. + + 1) Empty types dictionary is correct by definition: + >>> Types._check_types_dictionary({}, set()) + + 2) There are supertypes, but :typing requirement not specified + >>> a, b, c = to_names(["a", "b", "c"]) + >>> Types._check_types_dictionary({a: b, b: c}, set()) + Traceback (most recent call last): + ... + pddl.exceptions.PDDLValidationError: typing requirement is not specified, but types are used: 'b', 'c' + + 3) The `object` type cannot be a subtype: + >>> a = name_type("a") + >>> Types._check_types_dictionary({name_type("object"): a}, {Requirements.TYPING}) + Traceback (most recent call last): + ... + pddl.exceptions.PDDLValidationError: object must not have supertypes, but got 'object' is a subtype of 'a' + + 4) If cycles in the type hierarchy graph are present, an error is raised: + >>> a, b, c = to_names(["a", "b", "c"]) + >>> Types._check_types_dictionary({a: b, b: c, c: a}, {Requirements.TYPING}) + Traceback (most recent call last): + ... + pddl.exceptions.PDDLValidationError: cycle detected in the type hierarchy: a -> b -> c + + :param type_dict: the types dictionary + """ + if len(type_dict) == 0: + return + + # check typing requirement + supertypes = {t for t in type_dict.values() if t is not None} + if len(supertypes) > 0 and Requirements.TYPING not in requirements: + raise PDDLValidationError( + "typing requirement is not specified, but types are used: '" + + "', '".join(map(str, sorted(supertypes))) + + "'" + ) + + # check `object` type + object_name = name_type(Symbols.OBJECT.value) + if object_name in type_dict and type_dict[object_name] is not None: + object_supertype = type_dict[object_name] + raise PDDLValidationError( + f"object must not have supertypes, but got 'object' is a subtype of '{object_supertype}'" + ) + + # check cycles + cycle = find_cycle(type_dict) # type: ignore + if cycle is not None: + raise PDDLValidationError( + "cycle detected in the type hierarchy: " + " -> ".join(cycle) + ) diff --git a/pddl/core.py b/pddl/core.py index d9a1aff4..47b02d7d 100644 --- a/pddl/core.py +++ b/pddl/core.py @@ -15,24 +15,25 @@ It contains the class definitions to build and modify PDDL domains or problems. """ -from typing import AbstractSet, Collection, Dict, Optional, Sequence, Set, cast +from typing import AbstractSet, Collection, Dict, Optional, Sequence, cast -from pddl._validation import _check_constant_types, _check_types_in_has_terms_objects +from pddl._validation import ( + Types, + _check_constant_types, + _check_types_in_has_terms_objects, +) from pddl.custom_types import name as name_type from pddl.custom_types import namelike, to_names, to_names_types # noqa: F401 -from pddl.exceptions import PDDLValidationError from pddl.helpers.base import ( _typed_parameters, assert_, ensure, ensure_sequence, ensure_set, - find_cycle, ) from pddl.logic.base import Formula, TrueFormula, is_literal from pddl.logic.predicates import DerivedPredicate, Predicate from pddl.logic.terms import Constant, Term, Variable -from pddl.parser.symbols import Symbols from pddl.requirements import Requirements @@ -65,7 +66,7 @@ def __init__( """ self._name = name_type(name) self._requirements = ensure_set(requirements) - self._types = _Types(types, self._requirements) + self._types = Types(types, self._requirements) self._constants = ensure_set(constants) self._predicates = ensure_set(predicates) self._derived_predicates = ensure_set(derived_predicates) @@ -330,98 +331,3 @@ def __repr__(self) -> str: f"{type(self).__name__}({self.name}, parameters={', '.join(map(str, self.parameters))}, " f"precondition={self.precondition}, effect={self.effect})" ) - - -class _Types: - """A class for representing and managing the types available in a PDDL Domain.""" - - def __init__( - self, - types: Optional[Dict[namelike, Optional[namelike]]] = None, - requirements: Optional[AbstractSet[Requirements]] = None, - ) -> None: - """Initialize the Types object.""" - self._types = to_names_types(ensure(types, dict())) - - self._all_types = self._get_all_types() - self._check_types_dictionary(self._types, ensure_set(requirements)) - - @property - def raw(self) -> Dict[name_type, Optional[name_type]]: - """Get the raw types dictionary.""" - return self._types - - @property - def all_types(self) -> Set[name_type]: - """Get all available types.""" - return self._all_types - - def _get_all_types(self) -> Set[name_type]: - """Get all types supported by the domain.""" - if self._types is None: - return set() - result = set(self._types.keys()) | set(self._types.values()) - result.discard(None) - return cast(Set[name_type], result) - - @classmethod - def _check_types_dictionary( - cls, - type_dict: Dict[name_type, Optional[name_type]], - requirements: AbstractSet[Requirements], - ) -> None: - """ - Check the consistency of the types dictionary. - - 1) Empty types dictionary is correct by definition: - >>> _Types._check_types_dictionary({}, set()) - - 2) There are supertypes, but :typing requirement not specified - >>> a, b, c = to_names(["a", "b", "c"]) - >>> _Types._check_types_dictionary({a: b, b: c}, set()) - Traceback (most recent call last): - ... - pddl.exceptions.PDDLValidationError: typing requirement is not specified, but types are used: 'b', 'c' - - 3) The `object` type cannot be a subtype: - >>> a = name_type("a") - >>> _Types._check_types_dictionary({name_type("object"): a}, {Requirements.TYPING}) - Traceback (most recent call last): - ... - pddl.exceptions.PDDLValidationError: object must not have supertypes, but got 'object' is a subtype of 'a' - - 4) If cycles in the type hierarchy graph are present, an error is raised: - >>> a, b, c = to_names(["a", "b", "c"]) - >>> _Types._check_types_dictionary({a: b, b: c, c: a}, {Requirements.TYPING}) - Traceback (most recent call last): - ... - pddl.exceptions.PDDLValidationError: cycle detected in the type hierarchy: a -> b -> c - - :param type_dict: the types dictionary - """ - if len(type_dict) == 0: - return - - # check typing requirement - supertypes = {t for t in type_dict.values() if t is not None} - if len(supertypes) > 0 and Requirements.TYPING not in requirements: - raise PDDLValidationError( - "typing requirement is not specified, but types are used: '" - + "', '".join(map(str, sorted(supertypes))) - + "'" - ) - - # check `object` type - object_name = name_type(Symbols.OBJECT.value) - if object_name in type_dict and type_dict[object_name] is not None: - object_supertype = type_dict[object_name] - raise PDDLValidationError( - f"object must not have supertypes, but got 'object' is a subtype of '{object_supertype}'" - ) - - # check cycles - cycle = find_cycle(type_dict) # type: ignore - if cycle is not None: - raise PDDLValidationError( - "cycle detected in the type hierarchy: " + " -> ".join(cycle) - ) From 35f333bdccfb5e63fd796b51bc83b114df079218 Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 17:43:44 +0200 Subject: [PATCH 22/51] chore: return 'name' instead of str in domain/problem names property getters --- pddl/core.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pddl/core.py b/pddl/core.py index 47b02d7d..76255fd0 100644 --- a/pddl/core.py +++ b/pddl/core.py @@ -91,7 +91,7 @@ def _check_types_in_derived_predicates(self) -> None: _check_types_in_has_terms_objects(dp_list, self._types.all_types) @property - def name(self) -> str: + def name(self) -> name_type: """Get the name.""" return self._name @@ -189,7 +189,7 @@ def _check_consistency(self): ) @property - def name(self) -> str: + def name(self) -> name_type: """Get the name.""" return self._name @@ -210,13 +210,13 @@ def domain(self, domain: Domain) -> None: self._domain = domain @property - def domain_name(self) -> str: + def domain_name(self) -> name_type: """Get the domain name.""" if self._domain is not None: return self._domain.name assert_(self._domain_name is not None, "Domain name is not set.") - return cast(str, self._domain_name) + return cast(name_type, self._domain_name) @property def requirements(self) -> AbstractSet["Requirements"]: From 529d35d872267c7f83034d106b4664195b0b2d60 Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 17:45:43 +0200 Subject: [PATCH 23/51] chore: used 'validate' instead of 'assert_' in this way, PDDLValidationError is raised, rather than the generic AssertionError. --- pddl/_validation.py | 11 ++++++++++- pddl/core.py | 15 ++++++--------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/pddl/_validation.py b/pddl/_validation.py index 78b3aa6e..9a9f0e80 100644 --- a/pddl/_validation.py +++ b/pddl/_validation.py @@ -16,13 +16,22 @@ from pddl.custom_types import name as name_type from pddl.custom_types import namelike, to_names, to_names_types # noqa: F401 from pddl.exceptions import PDDLValidationError -from pddl.helpers.base import ensure, ensure_set, find_cycle +from pddl.helpers.base import check, ensure, ensure_set, find_cycle from pddl.logic import Constant, Predicate from pddl.logic.terms import Term from pddl.parser.symbols import ALL_SYMBOLS, Symbols from pddl.requirements import Requirements +def validate(condition: bool, message: str = "") -> None: + """ + Validate a condition regarding PDDL. + + If the condition is not satisfied, a PDDLValidationError is raised. + """ + check(condition, message, PDDLValidationError) + + def _find_inconsistencies_in_typed_terms( terms: Optional[Collection[Term]], all_types: Set[name_type] ) -> Optional[Tuple[Term, name_type]]: diff --git a/pddl/core.py b/pddl/core.py index 76255fd0..15df8554 100644 --- a/pddl/core.py +++ b/pddl/core.py @@ -15,12 +15,14 @@ It contains the class definitions to build and modify PDDL domains or problems. """ +from operator import xor from typing import AbstractSet, Collection, Dict, Optional, Sequence, cast from pddl._validation import ( Types, _check_constant_types, _check_types_in_has_terms_objects, + validate, ) from pddl.custom_types import name as name_type from pddl.custom_types import namelike, to_names, to_names_types # noqa: F401 @@ -170,7 +172,7 @@ def __init__( self._objects: AbstractSet[Constant] = ensure_set(objects) self._init: AbstractSet[Formula] = ensure_set(init) self._goal: Formula = ensure(goal, TrueFormula()) - assert_( + validate( all(map(is_literal, self.init)), "Not all formulas of initial condition are literals!", ) @@ -178,14 +180,9 @@ def __init__( self._check_consistency() def _check_consistency(self): - assert_( - self._domain is not None or self._domain_name is not None, - "At least one between 'domain' and 'domain_name' must be set.", - ) - assert_( - self._domain is None - or self._domain_name is None - or self._domain.name == self._domain_name + validate( + xor(self._domain is not None, self._domain_name is not None), + "Only one between 'domain' and 'domain_name' must be set.", ) @property From 0d264a8d016b95f807405f46b3a2415ce1e549bf Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 17:47:42 +0200 Subject: [PATCH 24/51] chore: add Problem.check method stub --- pddl/core.py | 16 ++++++++++++++++ tests/test_problem.py | 4 +--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/pddl/core.py b/pddl/core.py index 15df8554..0950b5a8 100644 --- a/pddl/core.py +++ b/pddl/core.py @@ -184,6 +184,22 @@ def _check_consistency(self): xor(self._domain is not None, self._domain_name is not None), "Only one between 'domain' and 'domain_name' must be set.", ) + if self._domain is not None: + self.check(self._domain) + + def check(self, domain: Domain) -> None: + """Check the problem definition against a domain definition.""" + validate( + self.domain_name == domain.name, + "Domain names don't match.", + ) + validate( + self.requirements == domain.requirements, + "Requirements don't match.", + ) + # TODO check objects + # TODO check init + # TODO check goal @property def name(self) -> name_type: diff --git a/tests/test_problem.py b/tests/test_problem.py index 7b25b38d..2428506a 100644 --- a/tests/test_problem.py +++ b/tests/test_problem.py @@ -13,7 +13,6 @@ """This module contains tests for a PDDL problem.""" import copy import pickle # nosec -from unittest.mock import MagicMock import pytest @@ -74,10 +73,9 @@ def test_build_simple_problem(): o1, o2, o3 = constants("o1 o2 o3") p = Predicate("p", x, y, z) q = Predicate("q", x, y, z) - domain = MagicMock() problem = Problem( "simple_problem", - domain, + domain_name="simple_domain", objects=[o1, o2, o3], init={p, Not(q)}, goal=p & q, From 03e632b2d3f6229216f32f79e1e6bc0f920b4f1d Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 18:14:38 +0200 Subject: [PATCH 25/51] chore: change the way requirements are set in a Problem object If the requirements set is given, use it. Otherwise, take the requirements from the domain. --- pddl/core.py | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/pddl/core.py b/pddl/core.py index 0950b5a8..0de50c5e 100644 --- a/pddl/core.py +++ b/pddl/core.py @@ -168,7 +168,9 @@ def __init__( self._name = name_type(name) self._domain: Optional[Domain] = domain self._domain_name = name_type(domain_name) if domain_name else None - self._requirements: AbstractSet[Requirements] = ensure_set(requirements) + self._requirements: AbstractSet[Requirements] = self._parse_requirements( + domain, requirements + ) self._objects: AbstractSet[Constant] = ensure_set(objects) self._init: AbstractSet[Formula] = ensure_set(init) self._goal: Formula = ensure(goal, TrueFormula()) @@ -179,7 +181,22 @@ def __init__( self._check_consistency() - def _check_consistency(self): + def _parse_requirements( + self, + domain: Optional[Domain], + requirements: Optional[Collection["Requirements"]], + ) -> AbstractSet[Requirements]: + """ + Parse the requirements. + + If the requirements set is given, use it. Otherwise, take the requirements from the domain. + """ + if requirements is not None or domain is None: + return ensure_set(requirements) + return domain.requirements + + def _check_consistency(self) -> None: + """Check consistency of the PDDL Problem instance object.""" validate( xor(self._domain is not None, self._domain_name is not None), "Only one between 'domain' and 'domain_name' must be set.", @@ -194,7 +211,7 @@ def check(self, domain: Domain) -> None: "Domain names don't match.", ) validate( - self.requirements == domain.requirements, + self.requirements is None or self.requirements == domain.requirements, "Requirements don't match.", ) # TODO check objects From f232127dc8af9cbb1a67d7871588233471e2ee9c Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 18:55:06 +0200 Subject: [PATCH 26/51] fix: update domain setter of Problem --- pddl/core.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/pddl/core.py b/pddl/core.py index 0de50c5e..3d9c07b4 100644 --- a/pddl/core.py +++ b/pddl/core.py @@ -232,12 +232,9 @@ def domain(self) -> Domain: @domain.setter def domain(self, domain: Domain) -> None: """Set the domain.""" - if self._domain_name is not None: - assert_( - self._domain_name == domain.name, - f"Domain names don't match. Expected {self._domain_name}, got {domain.name}.", - ) + self._domain_name = None self._domain = domain + self._check_consistency() @property def domain_name(self) -> name_type: From 1586db1cd1a2ab1d697bb256f593b7521555e42c Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 19:06:03 +0200 Subject: [PATCH 27/51] feat: add TypeChecker utility class and use it for checking (both domain and problem) constants --- pddl/_validation.py | 70 ++++++++++++++++++++++++++++++++++++-------- pddl/core.py | 26 ++++++++++------ tests/test_domain.py | 10 +++---- 3 files changed, 79 insertions(+), 27 deletions(-) diff --git a/pddl/_validation.py b/pddl/_validation.py index 9a9f0e80..7504dfda 100644 --- a/pddl/_validation.py +++ b/pddl/_validation.py @@ -11,13 +11,16 @@ # """This module defines validation functions for PDDL data structures.""" + +import functools +from collections.abc import Iterable from typing import AbstractSet, Collection, Dict, Optional, Set, Tuple, cast from pddl.custom_types import name as name_type from pddl.custom_types import namelike, to_names, to_names_types # noqa: F401 from pddl.exceptions import PDDLValidationError from pddl.helpers.base import check, ensure, ensure_set, find_cycle -from pddl.logic import Constant, Predicate +from pddl.logic import Predicate from pddl.logic.terms import Term from pddl.parser.symbols import ALL_SYMBOLS, Symbols from pddl.requirements import Requirements @@ -51,17 +54,6 @@ def _find_inconsistencies_in_typed_terms( return None -def _check_constant_types( - constants: Optional[Collection[Constant]], all_types: Set[name_type] -) -> None: - check_result = _find_inconsistencies_in_typed_terms(constants, all_types) - if check_result is not None: - constant, type_tag = check_result - raise PDDLValidationError( - f"type {repr(type_tag)} of constant {repr(constant)} is not in available types {all_types}" - ) - - def _check_types_in_has_terms_objects( has_terms_objects: Optional[Collection[Predicate]], all_types: Set[name_type], @@ -93,12 +85,15 @@ def __init__( self, types: Optional[Dict[namelike, Optional[namelike]]] = None, requirements: Optional[AbstractSet[Requirements]] = None, + skip_checks: bool = False, ) -> None: """Initialize the Types object.""" self._types = to_names_types(ensure(types, dict())) self._all_types = self._get_all_types() - self._check_types_dictionary(self._types, ensure_set(requirements)) + + if not skip_checks: + self._check_types_dictionary(self._types, ensure_set(requirements)) @property def raw(self) -> Dict[name_type, Optional[name_type]]: @@ -179,3 +174,52 @@ def _check_types_dictionary( raise PDDLValidationError( "cycle detected in the type hierarchy: " + " -> ".join(cycle) ) + + +class TypeChecker: + """Implementation of a type checker for domains and problems.""" + + def __init__( + self, types: Types, requirements: Optional[AbstractSet[Requirements]] = None + ) -> None: + """Initialize the type checker.""" + self._types = types + self._requirements = ensure_set(requirements) + + @property + def has_typing(self) -> bool: + """Check if the typing requirement is specified.""" + return Requirements.TYPING in self._requirements + + def _check_typing_requirement(self, type_tags: Collection[name_type]) -> None: + """Check that the typing requirement is specified.""" + if not self.has_typing and len(type_tags) > 0: + raise PDDLValidationError( + f"typing requirement is not specified, but the following types were used: {type_tags}" + ) + + def _check_types_are_available( + self, type_tags: Collection[name_type], what: str + ) -> None: + """Check that the types are available in the domain.""" + if not self._types.all_types.issuperset(type_tags): + raise PDDLValidationError( + f"types {repr(type_tags)} of {what} are not in available types {self._types.all_types}" + ) + + @functools.singledispatchmethod # type: ignore + def check_type(self, obj: object): + """Check types annotations of PDDL data structures.""" + raise NotImplementedError(f"cannot check PDDL types of {type(obj)}") + + @check_type.register + def _(self, objects: Iterable) -> None: + """Check the types of collections of objects.""" + for obj in objects: + self.check_type(obj) + + @check_type.register + def _(self, term: Term) -> None: + """Check types annotations of PDDL data structures.""" + self._check_typing_requirement(term.type_tags) + self._check_types_are_available(term.type_tags, f"term {repr(term)}") diff --git a/pddl/core.py b/pddl/core.py index 3d9c07b4..36eefcf5 100644 --- a/pddl/core.py +++ b/pddl/core.py @@ -19,8 +19,8 @@ from typing import AbstractSet, Collection, Dict, Optional, Sequence, cast from pddl._validation import ( + TypeChecker, Types, - _check_constant_types, _check_types_in_has_terms_objects, validate, ) @@ -78,7 +78,8 @@ def __init__( def _check_consistency(self) -> None: """Check consistency of a domain instance object.""" - _check_constant_types(self._constants, self._types.all_types) + checker = TypeChecker(self._types, self.requirements) + checker.check_type(self.constants) _check_types_in_has_terms_objects(self._predicates, self._types.all_types) _check_types_in_has_terms_objects(self._actions, self._types.all_types) # type: ignore self._check_types_in_derived_predicates() @@ -168,9 +169,9 @@ def __init__( self._name = name_type(name) self._domain: Optional[Domain] = domain self._domain_name = name_type(domain_name) if domain_name else None - self._requirements: AbstractSet[Requirements] = self._parse_requirements( - domain, requirements - ) + self._requirements: Optional[ + AbstractSet[Requirements] + ] = self._parse_requirements(domain, requirements) self._objects: AbstractSet[Constant] = ensure_set(objects) self._init: AbstractSet[Formula] = ensure_set(init) self._goal: Formula = ensure(goal, TrueFormula()) @@ -185,14 +186,14 @@ def _parse_requirements( self, domain: Optional[Domain], requirements: Optional[Collection["Requirements"]], - ) -> AbstractSet[Requirements]: + ) -> Optional[AbstractSet[Requirements]]: """ Parse the requirements. If the requirements set is given, use it. Otherwise, take the requirements from the domain. """ if requirements is not None or domain is None: - return ensure_set(requirements) + return set(requirements) if requirements is not None else None return domain.requirements def _check_consistency(self) -> None: @@ -214,7 +215,9 @@ def check(self, domain: Domain) -> None: self.requirements is None or self.requirements == domain.requirements, "Requirements don't match.", ) - # TODO check objects + types = Types(domain.types, domain.requirements, skip_checks=True) # type: ignore + type_checker = TypeChecker(types, domain.requirements) + type_checker.check_type(self.objects) # TODO check init # TODO check goal @@ -248,7 +251,12 @@ def domain_name(self) -> name_type: @property def requirements(self) -> AbstractSet["Requirements"]: """Get the requirements.""" - return self._requirements + if self._domain is not None: + return self._domain.requirements + + if self._requirements is None: + raise AttributeError("neither requirements nor domain are set.") + return cast(AbstractSet[Requirements], self._requirements) @property def objects(self) -> AbstractSet["Constant"]: diff --git a/tests/test_domain.py b/tests/test_domain.py index d90ccadd..7aff564e 100644 --- a/tests/test_domain.py +++ b/tests/test_domain.py @@ -117,9 +117,9 @@ def test_constants_type_not_available() -> None: with pytest.raises( PDDLValidationError, - match=f"type 't1' of constant {re.escape(repr(a))} is not in available types {{'{my_type}'}}", + match=f"types {{'t1'}} of term {re.escape(repr(a))} are not in available types {{'{my_type}'}}", ): - Domain("test", constants={a}, types=type_set) # type: ignore + Domain("test", requirements={Requirements.TYPING}, constants={a}, types=type_set) # type: ignore def test_predicate_variable_type_not_available() -> None: @@ -135,7 +135,7 @@ def test_predicate_variable_type_not_available() -> None: match=rf"type '(t1|t2)' of term {re.escape(repr(x))} in atomic expression {re.escape(repr(p))} is not in " f"available types {{'{my_type}'}}", ): - Domain("test", predicates={p}, types=type_set) # type: ignore + Domain("test", requirements={Requirements.TYPING}, predicates={p}, types=type_set) # type: ignore def test_action_parameter_type_not_available() -> None: @@ -151,7 +151,7 @@ def test_action_parameter_type_not_available() -> None: match=rf"type '(t1|t2)' of term {re.escape(repr(x))} in atomic expression {re.escape(repr(action))} is not in " f"available types {{'{my_type}'}}", ): - Domain("test", actions={action}, types=type_set) # type: ignore + Domain("test", requirements={Requirements.TYPING}, actions={action}, types=type_set) # type: ignore def test_derived_predicate_type_not_available() -> None: @@ -168,4 +168,4 @@ def test_derived_predicate_type_not_available() -> None: match=rf"type '(t1|t2)' of term {re.escape(repr(x))} in atomic expression {re.escape(repr(p))} is not in " f"available types {{'{my_type}'}}", ): - Domain("test", derived_predicates={dp}, types=type_set) # type: ignore + Domain("test", requirements={Requirements.TYPING}, derived_predicates={dp}, types=type_set) # type: ignore From e879efba93b8626834e0335fb428423d33d3634d Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 19:26:10 +0200 Subject: [PATCH 28/51] fix: improve Problem initialize regarding requirements and domain_name vs domain Problem objects can be instantiated in two ways: either with a domain object, or without it. If with a domain object, then both the attributes/args domain_name and requirements must be validated against the provided domain. Otherwise, the provided arguments are used to set the attributes. The domain setter will reset both requirements and domain_name attributes. The property getter of requirements will return empty set even when it was not specified (therefore, interpreting absence of requirements section as empty set of requirements). --- pddl/core.py | 59 +++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 47 insertions(+), 12 deletions(-) diff --git a/pddl/core.py b/pddl/core.py index 36eefcf5..ff7b89dc 100644 --- a/pddl/core.py +++ b/pddl/core.py @@ -16,7 +16,7 @@ It contains the class definitions to build and modify PDDL domains or problems. """ from operator import xor -from typing import AbstractSet, Collection, Dict, Optional, Sequence, cast +from typing import AbstractSet, Collection, Dict, Optional, Sequence, Tuple, cast from pddl._validation import ( TypeChecker, @@ -29,6 +29,7 @@ from pddl.helpers.base import ( _typed_parameters, assert_, + check, ensure, ensure_sequence, ensure_set, @@ -149,7 +150,7 @@ def __init__( self, name: namelike, domain: Optional[Domain] = None, - domain_name: Optional[str] = None, + domain_name: Optional[namelike] = None, requirements: Optional[Collection["Requirements"]] = None, objects: Optional[Collection["Constant"]] = None, init: Optional[Collection[Formula]] = None, @@ -167,8 +168,11 @@ def __init__( :param goal: the goal condition. """ self._name = name_type(name) - self._domain: Optional[Domain] = domain - self._domain_name = name_type(domain_name) if domain_name else None + self._domain: Optional[Domain] + self._domain_name: name_type + self._domain, self._domain_name = self._parse_domain_and_domain_name( + domain, domain_name + ) self._requirements: Optional[ AbstractSet[Requirements] ] = self._parse_requirements(domain, requirements) @@ -182,18 +186,51 @@ def __init__( self._check_consistency() + def _parse_domain_and_domain_name( + self, + domain: Optional[Domain], + domain_name: Optional[namelike], + ) -> Tuple[Optional[Domain], name_type]: + """ + Parse the domain and domain name. + + If the domain is given, use it. Otherwise, take the domain from the domain name. + """ + if domain is not None and domain_name is not None: + check( + domain.name == domain_name, + f"got both domain and domain_name, but domain_name differs: {domain.name} != {domain_name}", + exception_cls=ValueError, + ) + return domain, name_type(domain_name) + if domain is not None: + return domain, domain.name + if domain_name is not None: + return None, name_type(domain_name) + raise ValueError("Either domain or domain_name must be given.") + def _parse_requirements( self, domain: Optional[Domain], - requirements: Optional[Collection["Requirements"]], + requirements: Optional[Collection[Requirements]], ) -> Optional[AbstractSet[Requirements]]: """ Parse the requirements. If the requirements set is given, use it. Otherwise, take the requirements from the domain. """ - if requirements is not None or domain is None: - return set(requirements) if requirements is not None else None + if requirements is None: + return None if domain is None else domain.requirements + + # requirements is not None + if domain is None: + return set(cast(Collection[Requirements], requirements)) + + check( + requirements == domain.requirements, + f"got both requirements and domain, but requirements differ: {requirements} != {domain.requirements}", + exception_cls=ValueError, + ) return domain.requirements def _check_consistency(self) -> None: @@ -212,7 +249,7 @@ def check(self, domain: Domain) -> None: "Domain names don't match.", ) validate( - self.requirements is None or self.requirements == domain.requirements, + self._requirements is None or self._requirements == domain.requirements, "Requirements don't match.", ) types = Types(domain.types, domain.requirements, skip_checks=True) # type: ignore @@ -235,7 +272,7 @@ def domain(self) -> Domain: @domain.setter def domain(self, domain: Domain) -> None: """Set the domain.""" - self._domain_name = None + self._domain_name = domain.name self._domain = domain self._check_consistency() @@ -254,9 +291,7 @@ def requirements(self) -> AbstractSet["Requirements"]: if self._domain is not None: return self._domain.requirements - if self._requirements is None: - raise AttributeError("neither requirements nor domain are set.") - return cast(AbstractSet[Requirements], self._requirements) + return self._requirements if self._requirements is not None else set() @property def objects(self) -> AbstractSet["Constant"]: From 771c6fc972e4160e609b4833db75c8c25f501ef6 Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Wed, 7 Jun 2023 19:28:34 +0200 Subject: [PATCH 29/51] test: refactor parametrized tests - The parser tests are split in two modules: one for domain's, the other for problem's parser tests. - The parametrization over PDDL fixture files of the parser tests is changed. Now we iterate over all problems, and for each problem we also parse the respective domain, and then we check the validity of the problem against the domain (via the implicit call to the Problem.check method, triggered by the Problem.domain property setter). --- tests/conftest.py | 10 +++- tests/test_parser/test_domain.py | 42 +------------- tests/test_parser/test_parametrized.py | 80 ++++++++++++++++++++++++++ tests/test_parser/test_problem.py | 33 ----------- 4 files changed, 90 insertions(+), 75 deletions(-) create mode 100644 tests/test_parser/test_parametrized.py diff --git a/tests/conftest.py b/tests/conftest.py index fb82d2cb..ffc3f5dd 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -12,6 +12,7 @@ """This module contains the configurations for the tests.""" import inspect +import itertools from pathlib import Path import mistune @@ -67,7 +68,14 @@ FIXTURES_PDDL_FILES / domain_name / "domain.pddl" for domain_name in DOMAIN_NAMES ] -PROBLEM_FILES = [*FIXTURES_PDDL_FILES.glob("./**/p*.pddl")] +PROBLEM_FILES = list( + itertools.chain( + *[ + (FIXTURES_PDDL_FILES / domain_name).rglob("p*.pddl") + for domain_name in DOMAIN_NAMES + ] + ) +) @pytest.fixture(scope="session") diff --git a/tests/test_parser/test_domain.py b/tests/test_parser/test_domain.py index 390b6e10..2475f4a3 100644 --- a/tests/test_parser/test_domain.py +++ b/tests/test_parser/test_domain.py @@ -11,54 +11,14 @@ # """This module contains the tests for the domain parser.""" -from pathlib import Path from textwrap import dedent import lark import pytest -from pytest import lazy_fixture # type:ignore # noqa -from pddl.core import Domain from pddl.parser.domain import DomainParser from pddl.parser.symbols import Symbols -from tests.conftest import ( - BLOCKSWORLD_FILES, - BLOCKSWORLD_FOND_FILES, - DOMAIN_FILES, - TEXT_SYMBOLS, - TRIANGLE_FILES, -) - - -@pytest.mark.parametrize("pddl_file", DOMAIN_FILES) -def test_domain_parser(domain_parser, pddl_file: Path): - """Test only that the domain parsing works for all the fixtures.""" - domain_parser(pddl_file.read_text()) - - -@pytest.mark.parametrize( - "pddl_file,expected_domain", - [ - ( - BLOCKSWORLD_FILES / "domain.pddl", - lazy_fixture("blocksworld_domain"), # type:ignore - ), - ( - TRIANGLE_FILES / "domain.pddl", - lazy_fixture("triangle_tireworld_domain"), # type:ignore - ), - ( - BLOCKSWORLD_FOND_FILES / "domain.pddl", - lazy_fixture("blocksworld_fond_domain"), # type:ignore - ), - ], -) -def test_check_domain_parser_output(domain_parser, pddl_file: Path, expected_domain): - """Test domain parsing.""" - actual_domain = domain_parser(pddl_file.read_text()) - - assert isinstance(actual_domain, Domain) - assert actual_domain == expected_domain +from tests.conftest import TEXT_SYMBOLS def test_hierarchical_types() -> None: diff --git a/tests/test_parser/test_parametrized.py b/tests/test_parser/test_parametrized.py new file mode 100644 index 00000000..35c75889 --- /dev/null +++ b/tests/test_parser/test_parametrized.py @@ -0,0 +1,80 @@ +# +# Copyright 2021-2023 WhiteMech +# +# ------------------------------ +# +# This file is part of pddl. +# +# Use of this source code is governed by an MIT-style +# license that can be found in the LICENSE file or at +# https://opensource.org/licenses/MIT. +# + +"""This module contains parametrized tests for both domains and problems parsers.""" +from pathlib import Path + +import pytest +from pytest import lazy_fixture # type:ignore # noqa + +from pddl.core import Domain, Problem +from tests.conftest import ( + BLOCKSWORLD_FILES, + BLOCKSWORLD_FOND_FILES, + PROBLEM_FILES, + TRIANGLE_FILES, +) + + +@pytest.mark.parametrize("pddl_file", PROBLEM_FILES) +def test_problem_parser(domain_parser, problem_parser, pddl_file: Path): + """Test problem parsing and validation works for all the fixtures.""" + problem = problem_parser(pddl_file.read_text()) + domain_file = pddl_file.parent / "domain.pddl" + domain = domain_parser(domain_file.read_text()) + problem.check(domain) + + +@pytest.mark.parametrize( + "pddl_file,expected_problem", + [ + ( + BLOCKSWORLD_FILES / "p01.pddl", + lazy_fixture("blocksworld_problem_01"), # type:ignore + ), + ( + BLOCKSWORLD_FOND_FILES / "p01.pddl", + lazy_fixture("blocksworld_fond_01"), # type:ignore + ), + ], +) +def test_check_problem_parser_output(problem_parser, pddl_file: Path, expected_problem): + """Test problem parsing.""" + actual_problem = problem_parser(pddl_file.read_text()) + + assert isinstance(actual_problem, Problem) + assert actual_problem == expected_problem + + +@pytest.mark.parametrize( + "pddl_file,expected_domain", + [ + ( + BLOCKSWORLD_FILES / "domain.pddl", + lazy_fixture("blocksworld_domain"), # type:ignore + ), + ( + TRIANGLE_FILES / "domain.pddl", + lazy_fixture("triangle_tireworld_domain"), # type:ignore + ), + ( + BLOCKSWORLD_FOND_FILES / "domain.pddl", + lazy_fixture("blocksworld_fond_domain"), # type:ignore + ), + ], +) +def test_check_domain_parser_output(domain_parser, pddl_file: Path, expected_domain): + """Test domain parsing.""" + actual_domain = domain_parser(pddl_file.read_text()) + + assert isinstance(actual_domain, Domain) + assert actual_domain == expected_domain diff --git a/tests/test_parser/test_problem.py b/tests/test_parser/test_problem.py index 26467079..ba57a13c 100644 --- a/tests/test_parser/test_problem.py +++ b/tests/test_parser/test_problem.py @@ -11,43 +11,10 @@ # """This module contains the tests for the domain parser.""" -from pathlib import Path from textwrap import dedent -import pytest -from pytest import lazy_fixture # type:ignore # noqa - -from pddl.core import Problem from pddl.parser.problem import ProblemParser from pddl.requirements import Requirements -from tests.conftest import BLOCKSWORLD_FILES, BLOCKSWORLD_FOND_FILES, PROBLEM_FILES - - -@pytest.mark.parametrize("pddl_file", PROBLEM_FILES) -def test_problem_parser(problem_parser, pddl_file: Path): - """Test only that the problem parsing works for all the fixtures.""" - problem_parser(pddl_file.read_text()) - - -@pytest.mark.parametrize( - "pddl_file,expected_problem", - [ - ( - BLOCKSWORLD_FILES / "p01.pddl", - lazy_fixture("blocksworld_problem_01"), # type:ignore - ), - ( - BLOCKSWORLD_FOND_FILES / "p01.pddl", - lazy_fixture("blocksworld_fond_01"), # type:ignore - ), - ], -) -def test_check_problem_parser_output(problem_parser, pddl_file: Path, expected_problem): - """Test problem parsing.""" - actual_problem = problem_parser(pddl_file.read_text()) - - assert isinstance(actual_problem, Problem) - assert actual_problem == expected_problem def test_problem_requirements_section_parsed() -> None: From 2d35e8ccdb46765734ea7b7861a87e5dfe47f0e2 Mon Sep 17 00:00:00 2001 From: Marco Favorito Date: Wed, 7 Jun 2023 20:58:10 +0200 Subject: [PATCH 30/51] build: bump minimum Python interpreter supported to 3.8 The main motivation is that Python 3.7 is going to reach the end of life at the end of June 2023. Another reason is that we are going to use some language features available only for >=3.8 (e.g. functools.singledispatchmethod). --- .github/workflows/docs.yml | 2 +- .github/workflows/test.yml | 2 +- Pipfile.lock | 22 +++++++++++++++++++--- README.md | 2 +- setup.cfg | 2 +- setup.py | 3 +-- 6 files changed, 24 insertions(+), 9 deletions(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index bc5fcdcd..5efd2f91 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -13,7 +13,7 @@ jobs: strategy: matrix: os: [ubuntu-latest] - python-version: [3.7] + python-version: [3.8] timeout-minutes: 30 diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 3711e54d..c9da0db3 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -13,7 +13,7 @@ jobs: strategy: matrix: os: [ubuntu-latest] - python-version: ["3.7", "3.8", "3.9", "3.10", "3.11"] + python-version: ["3.8", "3.9", "3.10", "3.11"] timeout-minutes: 30 diff --git a/Pipfile.lock b/Pipfile.lock index 7399d32e..38f4629b 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -297,6 +297,14 @@ "markers": "python_version >= '3.5'", "version": "==0.6.2" }, + "exceptiongroup": { + "hashes": [ + "sha256:232c37c63e4f682982c8b6459f33a8981039e5fb8756b2074364e5055c498c9e", + "sha256:d484c3090ba2889ae2928419117447a14daf3c1231d5e30d0aae34f354f01785" + ], + "markers": "python_version < '3.11'", + "version": "==1.1.1" + }, "executing": { "hashes": [ "sha256:0314a69e37426e3608aada02473b4161d4caf5a4b244d1d0c48072b8fee7bacc", @@ -1058,6 +1066,14 @@ "markers": "python_version >= '2.6' and python_version not in '3.0, 3.1, 3.2, 3.3'", "version": "==0.10.2" }, + "tomli": { + "hashes": [ + "sha256:939de3e7a6161af0c887ef91b7d41a53e7c5a1ca976325f429cb46ea9bc30ecc", + "sha256:de526c12914f0c550d15924c62d72abc48d6fe7364aa87328337a31007fe8a4f" + ], + "markers": "python_version < '3.11'", + "version": "==2.0.1" + }, "tox": { "hashes": [ "sha256:740f5209d0dec19451b951ee5b1cce4a207acdc7357af84dbc8ec35bcf2c454e", @@ -1100,11 +1116,11 @@ }, "urllib3": { "hashes": [ - "sha256:61717a1095d7e155cdb737ac7bb2f4324a858a1e2e6466f6d03ff630ca68d3cc", - "sha256:d055c2f9d38dc53c808f6fdc8eab7360b6fdbbde02340ed25cfbcd817c62469e" + "sha256:48e7fafa40319d358848e1bc6809b208340fafe2096f1725d05d67443d0483d1", + "sha256:bee28b5e56addb8226c96f7f13ac28cb4c301dd5ea8a6ca179c0b9835e032825" ], "markers": "python_version >= '3.7'", - "version": "==2.0.2" + "version": "==2.0.3" }, "virtualenv": { "hashes": [ diff --git a/README.md b/README.md index 1f1a87d7..01f8d91e 100644 --- a/README.md +++ b/README.md @@ -199,7 +199,7 @@ If you want to contribute, here's how to set up your development environment. - Install [Pipenv](https://pipenv-fork.readthedocs.io/en/latest/) - Clone the repository: `git clone https://github.com/AI-Planning/pddl.git && cd pddl` -- Install development dependencies: `pipenv shell --python 3.7 && pipenv install --dev` +- Install development dependencies: `pipenv shell --python 3.8 && pipenv install --dev` ## Tests diff --git a/setup.cfg b/setup.cfg index 654aba31..5b5a9cf3 100644 --- a/setup.cfg +++ b/setup.cfg @@ -36,7 +36,7 @@ line_length=88 [mypy] -python_version = 3.7 +python_version = 3.8 strict_optional = True # Per-module options: diff --git a/setup.py b/setup.py index b7e32056..7356aea3 100644 --- a/setup.py +++ b/setup.py @@ -49,13 +49,12 @@ 'License :: OSI Approved :: MIT License', 'Natural Language :: English', 'Programming Language :: Python :: 3', - 'Programming Language :: Python :: 3.7', 'Programming Language :: Python :: 3.8', 'Programming Language :: Python :: 3.9', 'Programming Language :: Python :: 3.10', 'Programming Language :: Python :: 3.11', ], - python_requires='>=3.7', + python_requires='>=3.8', install_requires=install_requires, license=about["__license__"], include_package_data=True, From 190a7c513b31bcaceab68ecb2032005c15494193 Mon Sep 17 00:00:00 2001 From: Marco Favorito Date: Wed, 7 Jun 2023 21:33:59 +0200 Subject: [PATCH 31/51] fix: minor fixes to requirements handling in core module --- pddl/core.py | 7 +------ pddl/requirements.py | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/pddl/core.py b/pddl/core.py index ff7b89dc..a982dd75 100644 --- a/pddl/core.py +++ b/pddl/core.py @@ -15,7 +15,6 @@ It contains the class definitions to build and modify PDDL domains or problems. """ -from operator import xor from typing import AbstractSet, Collection, Dict, Optional, Sequence, Tuple, cast from pddl._validation import ( @@ -227,7 +226,7 @@ def _parse_requirements( return set(cast(Collection[Requirements], requirements)) check( - requirements == domain.requirements, + ensure_set(requirements) == domain.requirements, f"got both requirements and domain, but requirements differ: {requirements} != {domain.requirements}", exception_cls=ValueError, ) @@ -235,10 +234,6 @@ def _parse_requirements( def _check_consistency(self) -> None: """Check consistency of the PDDL Problem instance object.""" - validate( - xor(self._domain is not None, self._domain_name is not None), - "Only one between 'domain' and 'domain_name' must be set.", - ) if self._domain is not None: self.check(self._domain) diff --git a/pddl/requirements.py b/pddl/requirements.py index 1d67582d..22622d06 100644 --- a/pddl/requirements.py +++ b/pddl/requirements.py @@ -52,7 +52,7 @@ def __str__(self) -> str: def __repr__(self) -> str: """Get an unambiguous representation.""" - return f"Requirements{self.name}" + return f"Requirements.{self.name}" def __lt__(self, other): """Compare with another object.""" From 4376e540f611b1a381b9e2fce130f562c5ddcf5f Mon Sep 17 00:00:00 2001 From: Marco Favorito Date: Wed, 7 Jun 2023 22:34:54 +0200 Subject: [PATCH 32/51] feat: add validation of domain/problem predicates --- pddl/_validation.py | 71 ++++++++++++++++++++++++++++++++++++++++++-- pddl/core.py | 8 ++--- tests/test_domain.py | 2 +- 3 files changed, 74 insertions(+), 7 deletions(-) diff --git a/pddl/_validation.py b/pddl/_validation.py index 7504dfda..56647a4e 100644 --- a/pddl/_validation.py +++ b/pddl/_validation.py @@ -21,6 +21,15 @@ from pddl.exceptions import PDDLValidationError from pddl.helpers.base import check, ensure, ensure_set, find_cycle from pddl.logic import Predicate +from pddl.logic.base import ( + BinaryOp, + FalseFormula, + QuantifiedCondition, + TrueFormula, + UnaryOp, +) +from pddl.logic.effects import AndEffect, Forall, When +from pddl.logic.predicates import DerivedPredicate, EqualTo from pddl.logic.terms import Term from pddl.parser.symbols import ALL_SYMBOLS, Symbols from pddl.requirements import Requirements @@ -204,7 +213,7 @@ def _check_types_are_available( """Check that the types are available in the domain.""" if not self._types.all_types.issuperset(type_tags): raise PDDLValidationError( - f"types {repr(type_tags)} of {what} are not in available types {self._types.all_types}" + f"types {repr(set(sorted(type_tags)))} of {what} are not in available types {self._types.all_types}" ) @functools.singledispatchmethod # type: ignore @@ -220,6 +229,64 @@ def _(self, objects: Iterable) -> None: @check_type.register def _(self, term: Term) -> None: - """Check types annotations of PDDL data structures.""" + """Check types annotations of a PDDL term.""" self._check_typing_requirement(term.type_tags) self._check_types_are_available(term.type_tags, f"term {repr(term)}") + + @check_type.register + def _(self, predicate: Predicate) -> None: + """Check types annotations of a PDDL predicate.""" + self.check_type(predicate.terms) + + @check_type.register + def _(self, equal_to: EqualTo) -> None: + """Check types annotations of a PDDL equal-to atomic formula.""" + self.check_type(equal_to.left) + self.check_type(equal_to.right) + + @check_type.register + def _(self, derived_predicate: DerivedPredicate) -> None: + """Check types annotations of a PDDL derived predicate.""" + self.check_type(derived_predicate.predicate) + self.check_type(derived_predicate.condition) + + @check_type.register + def _(self, formula: UnaryOp) -> None: + """Check types annotations of a PDDL unary operator.""" + self.check_type(formula.argument) + + @check_type.register + def _(self, formula: BinaryOp) -> None: + """Check types annotations of a PDDL binary operator.""" + self.check_type(formula.operands) + + @check_type.register + def _(self, formula: TrueFormula) -> None: + """Check types annotations of a PDDL true formula.""" + + @check_type.register + def _(self, formula: FalseFormula) -> None: + """Check types annotations of a PDDL false formula.""" + + @check_type.register + def _(self, formula: QuantifiedCondition) -> None: + """Check types annotations of a PDDL quantified condition.""" + self.check_type(formula.variables) + self.check_type(formula.condition) + + @check_type.register + def _(self, effect: AndEffect) -> None: + """Check types annotations of a PDDL and-effect.""" + self.check_type(effect.operands) + + @check_type.register + def _(self, effect: When) -> None: + """Check types annotations of a PDDL when-effect.""" + self.check_type(effect.condition) + self.check_type(effect.effect) + + @check_type.register + def _(self, effect: Forall) -> None: + """Check types annotations of a PDDL forall-effect.""" + self.check_type(effect.variables) + self.check_type(effect.effect) diff --git a/pddl/core.py b/pddl/core.py index a982dd75..c1874c10 100644 --- a/pddl/core.py +++ b/pddl/core.py @@ -79,8 +79,8 @@ def __init__( def _check_consistency(self) -> None: """Check consistency of a domain instance object.""" checker = TypeChecker(self._types, self.requirements) - checker.check_type(self.constants) - _check_types_in_has_terms_objects(self._predicates, self._types.all_types) + checker.check_type(self._constants) + checker.check_type(self._predicates) _check_types_in_has_terms_objects(self._actions, self._types.all_types) # type: ignore self._check_types_in_derived_predicates() @@ -250,8 +250,8 @@ def check(self, domain: Domain) -> None: types = Types(domain.types, domain.requirements, skip_checks=True) # type: ignore type_checker = TypeChecker(types, domain.requirements) type_checker.check_type(self.objects) - # TODO check init - # TODO check goal + type_checker.check_type(self.init) + type_checker.check_type(self.goal) @property def name(self) -> name_type: diff --git a/tests/test_domain.py b/tests/test_domain.py index 7aff564e..90eedb3f 100644 --- a/tests/test_domain.py +++ b/tests/test_domain.py @@ -132,7 +132,7 @@ def test_predicate_variable_type_not_available() -> None: with pytest.raises( PDDLValidationError, - match=rf"type '(t1|t2)' of term {re.escape(repr(x))} in atomic expression {re.escape(repr(p))} is not in " + match=rf"types {{'t1', 't2'}} of term {re.escape(repr(x))} are not in " f"available types {{'{my_type}'}}", ): Domain("test", requirements={Requirements.TYPING}, predicates={p}, types=type_set) # type: ignore From a5110221854f57fd6f9f2232efc013af6adfc898 Mon Sep 17 00:00:00 2001 From: Marco Favorito Date: Wed, 7 Jun 2023 22:57:40 +0200 Subject: [PATCH 33/51] feat: add type validation of domain actions --- pddl/_validation.py | 8 ++++++++ pddl/core.py | 1 + 2 files changed, 9 insertions(+) diff --git a/pddl/_validation.py b/pddl/_validation.py index 56647a4e..35500525 100644 --- a/pddl/_validation.py +++ b/pddl/_validation.py @@ -16,6 +16,7 @@ from collections.abc import Iterable from typing import AbstractSet, Collection, Dict, Optional, Set, Tuple, cast +from pddl.core import Action from pddl.custom_types import name as name_type from pddl.custom_types import namelike, to_names, to_names_types # noqa: F401 from pddl.exceptions import PDDLValidationError @@ -290,3 +291,10 @@ def _(self, effect: Forall) -> None: """Check types annotations of a PDDL forall-effect.""" self.check_type(effect.variables) self.check_type(effect.effect) + + @check_type.register + def _(self, action: Action) -> None: + """Check types annotations of a PDDL term.""" + self.check_type(action.parameters) + self.check_type(action.precondition) + self.check_type(action.effect) diff --git a/pddl/core.py b/pddl/core.py index c1874c10..f213c0b3 100644 --- a/pddl/core.py +++ b/pddl/core.py @@ -81,6 +81,7 @@ def _check_consistency(self) -> None: checker = TypeChecker(self._types, self.requirements) checker.check_type(self._constants) checker.check_type(self._predicates) + checker.check_type(self._actions) _check_types_in_has_terms_objects(self._actions, self._types.all_types) # type: ignore self._check_types_in_derived_predicates() From 3d02538ee1e2f441350c47140f464ddcaad50e53 Mon Sep 17 00:00:00 2001 From: Marco Favorito Date: Wed, 7 Jun 2023 23:03:11 +0200 Subject: [PATCH 34/51] refactor: move Action class in its own module core.action Needed to avoid circular imports. --- README.md | 3 +- pddl/_validation.py | 2 +- pddl/action.py | 102 ++++++++++++++++++ pddl/core.py | 95 +--------------- pddl/parser/domain.py | 3 +- .../fixtures/code_objects/blocksworld_fond.py | 3 +- .../code_objects/blocksworld_ipc08.py | 3 +- .../code_objects/triangle_tireworld.py | 3 +- tests/test_action.py | 2 +- tests/test_actions.py | 2 +- tests/test_domain.py | 3 +- 11 files changed, 121 insertions(+), 100 deletions(-) create mode 100644 pddl/action.py diff --git a/README.md b/README.md index 01f8d91e..0b4c9e5d 100644 --- a/README.md +++ b/README.md @@ -82,7 +82,8 @@ programmatically: ```python from pddl.logic import Predicate, constants, variables -from pddl.core import Domain, Problem, Action +from pddl.core import Domain, Problem +from pddl.action import Action from pddl.formatter import domain_to_string, problem_to_string from pddl.requirements import Requirements diff --git a/pddl/_validation.py b/pddl/_validation.py index 35500525..f728bd28 100644 --- a/pddl/_validation.py +++ b/pddl/_validation.py @@ -16,7 +16,7 @@ from collections.abc import Iterable from typing import AbstractSet, Collection, Dict, Optional, Set, Tuple, cast -from pddl.core import Action +from pddl.action import Action from pddl.custom_types import name as name_type from pddl.custom_types import namelike, to_names, to_names_types # noqa: F401 from pddl.exceptions import PDDLValidationError diff --git a/pddl/action.py b/pddl/action.py new file mode 100644 index 00000000..1db8f2b6 --- /dev/null +++ b/pddl/action.py @@ -0,0 +1,102 @@ +# +# Copyright 2021-2023 WhiteMech +# +# ------------------------------ +# +# This file is part of pddl. +# +# Use of this source code is governed by an MIT-style +# license that can be found in the LICENSE file or at +# https://opensource.org/licenses/MIT. +# + +"""This module contains the definition of the PDDL action class.""" +from typing import Optional, Sequence + +from pddl.custom_types import name as name_type +from pddl.custom_types import namelike +from pddl.helpers.base import _typed_parameters, ensure_sequence +from pddl.logic import Variable +from pddl.logic.base import Formula +from pddl.logic.terms import Term + + +class Action: + """A class for the PDDL Action.""" + + def __init__( + self, + name: namelike, + parameters: Sequence[Variable], + precondition: Optional[Formula] = None, + effect: Optional[Formula] = None, + ): + """ + Initialize the action. + + :param name: the action name. + :param parameters: the action parameters. + :param precondition: the action precondition. + :param effect: the action effect. + """ + self._name: str = name_type(name) + self._parameters: Sequence[Variable] = ensure_sequence(parameters) + self._precondition = precondition + self._effect = effect + + @property + def name(self) -> str: + """Get the name.""" + return self._name + + @property + def parameters(self) -> Sequence[Variable]: + """Get the parameters.""" + return self._parameters + + @property + def terms(self) -> Sequence[Term]: + """Get the terms.""" + return self.parameters + + @property + def precondition(self) -> Optional[Formula]: + """Get the precondition.""" + return self._precondition + + @property + def effect(self) -> Optional[Formula]: + """Get the effect.""" + return self._effect + + def __str__(self): + """Get the string.""" + operator_str = "(:action {0}\n".format(self.name) + operator_str += f" :parameters ({_typed_parameters(self.parameters)})\n" + if self.precondition is not None: + operator_str += f" :precondition {str(self.precondition)}\n" + if self.effect is not None: + operator_str += f" :effect {str(self.effect)}\n" + operator_str += ")" + return operator_str + + def __eq__(self, other): + """Check equality between two Actions.""" + return ( + isinstance(other, Action) + and self.name == other.name + and self.parameters == other.parameters + and self.precondition == other.precondition + and self.effect == other.effect + ) + + def __hash__(self): + """Get the hash.""" + return hash((self.name, self.parameters, self.precondition, self.effect)) + + def __repr__(self) -> str: + """Get an unambiguous string representation.""" + return ( + f"{type(self).__name__}({self.name}, parameters={', '.join(map(str, self.parameters))}, " + f"precondition={self.precondition}, effect={self.effect})" + ) diff --git a/pddl/core.py b/pddl/core.py index f213c0b3..15839149 100644 --- a/pddl/core.py +++ b/pddl/core.py @@ -15,7 +15,7 @@ It contains the class definitions to build and modify PDDL domains or problems. """ -from typing import AbstractSet, Collection, Dict, Optional, Sequence, Tuple, cast +from typing import AbstractSet, Collection, Dict, Optional, Tuple, cast from pddl._validation import ( TypeChecker, @@ -23,19 +23,13 @@ _check_types_in_has_terms_objects, validate, ) +from pddl.action import Action from pddl.custom_types import name as name_type from pddl.custom_types import namelike, to_names, to_names_types # noqa: F401 -from pddl.helpers.base import ( - _typed_parameters, - assert_, - check, - ensure, - ensure_sequence, - ensure_set, -) +from pddl.helpers.base import assert_, check, ensure, ensure_set from pddl.logic.base import Formula, TrueFormula, is_literal from pddl.logic.predicates import DerivedPredicate, Predicate -from pddl.logic.terms import Constant, Term, Variable +from pddl.logic.terms import Constant from pddl.requirements import Requirements @@ -316,84 +310,3 @@ def __eq__(self, other): and self.init == other.init and self.goal == other.goal ) - - -class Action: - """A class for the PDDL Action.""" - - def __init__( - self, - name: namelike, - parameters: Sequence[Variable], - precondition: Optional[Formula] = None, - effect: Optional[Formula] = None, - ): - """ - Initialize the action. - - :param name: the action name. - :param parameters: the action parameters. - :param precondition: the action precondition. - :param effect: the action effect. - """ - self._name: str = name_type(name) - self._parameters: Sequence[Variable] = ensure_sequence(parameters) - self._precondition = precondition - self._effect = effect - - @property - def name(self) -> str: - """Get the name.""" - return self._name - - @property - def parameters(self) -> Sequence[Variable]: - """Get the parameters.""" - return self._parameters - - @property - def terms(self) -> Sequence[Term]: - """Get the terms.""" - return self.parameters - - @property - def precondition(self) -> Optional[Formula]: - """Get the precondition.""" - return self._precondition - - @property - def effect(self) -> Optional[Formula]: - """Get the effect.""" - return self._effect - - def __str__(self): - """Get the string.""" - operator_str = "(:action {0}\n".format(self.name) - operator_str += f" :parameters ({_typed_parameters(self.parameters)})\n" - if self.precondition is not None: - operator_str += f" :precondition {str(self.precondition)}\n" - if self.effect is not None: - operator_str += f" :effect {str(self.effect)}\n" - operator_str += ")" - return operator_str - - def __eq__(self, other): - """Check equality between two Actions.""" - return ( - isinstance(other, Action) - and self.name == other.name - and self.parameters == other.parameters - and self.precondition == other.precondition - and self.effect == other.effect - ) - - def __hash__(self): - """Get the hash.""" - return hash((self.name, self.parameters, self.precondition, self.effect)) - - def __repr__(self) -> str: - """Get an unambiguous string representation.""" - return ( - f"{type(self).__name__}({self.name}, parameters={', '.join(map(str, self.parameters))}, " - f"precondition={self.precondition}, effect={self.effect})" - ) diff --git a/pddl/parser/domain.py b/pddl/parser/domain.py index b02d6821..1efacb7d 100644 --- a/pddl/parser/domain.py +++ b/pddl/parser/domain.py @@ -18,7 +18,8 @@ from lark import Lark, ParseError, Transformer -from pddl.core import Action, Domain +from pddl.action import Action +from pddl.core import Domain from pddl.custom_types import name from pddl.exceptions import PDDLMissingRequirementError, PDDLParsingError from pddl.helpers.base import assert_ diff --git a/tests/fixtures/code_objects/blocksworld_fond.py b/tests/fixtures/code_objects/blocksworld_fond.py index 216d1cb2..9ac80f3f 100644 --- a/tests/fixtures/code_objects/blocksworld_fond.py +++ b/tests/fixtures/code_objects/blocksworld_fond.py @@ -13,7 +13,8 @@ """This test module contains the fixtures for 'blocksworld-ipc08' domain and problem.""" import pytest -from pddl.core import Action, Domain, Problem +from pddl.action import Action +from pddl.core import Domain, Problem from pddl.logic import Constant from pddl.logic.base import And, OneOf from pddl.logic.effects import AndEffect, When diff --git a/tests/fixtures/code_objects/blocksworld_ipc08.py b/tests/fixtures/code_objects/blocksworld_ipc08.py index efd4c4fa..19f7963b 100644 --- a/tests/fixtures/code_objects/blocksworld_ipc08.py +++ b/tests/fixtures/code_objects/blocksworld_ipc08.py @@ -13,7 +13,8 @@ """This test module contains the fixtures for 'blocksworld-ipc08' domain and problem.""" import pytest -from pddl.core import Action, Domain, Problem +from pddl.action import Action +from pddl.core import Domain, Problem from pddl.logic.base import And, OneOf from pddl.logic.effects import AndEffect from pddl.logic.helpers import constants, variables diff --git a/tests/fixtures/code_objects/triangle_tireworld.py b/tests/fixtures/code_objects/triangle_tireworld.py index e50b3ecc..aa2377a1 100644 --- a/tests/fixtures/code_objects/triangle_tireworld.py +++ b/tests/fixtures/code_objects/triangle_tireworld.py @@ -13,7 +13,8 @@ """This test module contains the fixtures for 'triangle-tireworld' domain and problem.""" import pytest -from pddl.core import Action, Domain, Problem +from pddl.action import Action +from pddl.core import Domain, Problem from pddl.logic.base import And, OneOf from pddl.logic.effects import AndEffect from pddl.logic.helpers import constants, variables diff --git a/tests/test_action.py b/tests/test_action.py index 3b34463c..2d9f2e65 100644 --- a/tests/test_action.py +++ b/tests/test_action.py @@ -11,7 +11,7 @@ # """This module contains tests for a PDDL action.""" -from pddl.core import Action +from pddl.action import Action from pddl.logic import Predicate, Variable, variables from pddl.logic.base import ExistsCondition, ForallCondition, Imply, OneOf diff --git a/tests/test_actions.py b/tests/test_actions.py index 348501a1..83cfdc85 100644 --- a/tests/test_actions.py +++ b/tests/test_actions.py @@ -12,7 +12,7 @@ """This module contains tests for PDDL actions.""" -from pddl.core import Action +from pddl.action import Action class TestActionSimpleInitialization: diff --git a/tests/test_domain.py b/tests/test_domain.py index 90eedb3f..ecff6b5b 100644 --- a/tests/test_domain.py +++ b/tests/test_domain.py @@ -17,7 +17,8 @@ import pytest -from pddl.core import Action, Domain +from pddl.action import Action +from pddl.core import Domain from pddl.exceptions import PDDLValidationError from pddl.logic import Constant, Variable from pddl.logic.base import Not, TrueFormula From 617cfd70d1101102e7837219be947dd4b3f7655c Mon Sep 17 00:00:00 2001 From: Marco Favorito Date: Wed, 7 Jun 2023 23:19:24 +0200 Subject: [PATCH 35/51] test: fix validation exception message --- pddl/_validation.py | 2 +- tests/test_domain.py | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/pddl/_validation.py b/pddl/_validation.py index f728bd28..94fbe757 100644 --- a/pddl/_validation.py +++ b/pddl/_validation.py @@ -214,7 +214,7 @@ def _check_types_are_available( """Check that the types are available in the domain.""" if not self._types.all_types.issuperset(type_tags): raise PDDLValidationError( - f"types {repr(set(sorted(type_tags)))} of {what} are not in available types {self._types.all_types}" + f"types {sorted(type_tags)} of {what} are not in available types {self._types.all_types}" ) @functools.singledispatchmethod # type: ignore diff --git a/tests/test_domain.py b/tests/test_domain.py index ecff6b5b..08561d84 100644 --- a/tests/test_domain.py +++ b/tests/test_domain.py @@ -118,7 +118,7 @@ def test_constants_type_not_available() -> None: with pytest.raises( PDDLValidationError, - match=f"types {{'t1'}} of term {re.escape(repr(a))} are not in available types {{'{my_type}'}}", + match=rf"types \['t1'\] of term {re.escape(repr(a))} are not in available types {{'{my_type}'}}", ): Domain("test", requirements={Requirements.TYPING}, constants={a}, types=type_set) # type: ignore @@ -133,8 +133,7 @@ def test_predicate_variable_type_not_available() -> None: with pytest.raises( PDDLValidationError, - match=rf"types {{'t1', 't2'}} of term {re.escape(repr(x))} are not in " - f"available types {{'{my_type}'}}", + match=rf"types \['t1', 't2'\] of term {re.escape(repr(x))} are not in available types {{'{my_type}'}}", ): Domain("test", requirements={Requirements.TYPING}, predicates={p}, types=type_set) # type: ignore @@ -149,8 +148,7 @@ def test_action_parameter_type_not_available() -> None: with pytest.raises( PDDLValidationError, - match=rf"type '(t1|t2)' of term {re.escape(repr(x))} in atomic expression {re.escape(repr(action))} is not in " - f"available types {{'{my_type}'}}", + match=rf"types \['t1', 't2'\] of term {re.escape(repr(x))} are not in available types {{'{my_type}'}}", ): Domain("test", requirements={Requirements.TYPING}, actions={action}, types=type_set) # type: ignore From bdddfbf79264709229fa0ba265069bce56643d4f Mon Sep 17 00:00:00 2001 From: Marco Favorito Date: Wed, 7 Jun 2023 23:19:47 +0200 Subject: [PATCH 36/51] feat: add keyword check --- pddl/_validation.py | 32 +++++++++++++++++++++++++++++++- pddl/parser/types_index.py | 29 ++++------------------------- tests/test_parser/test_domain.py | 6 ++---- 3 files changed, 37 insertions(+), 30 deletions(-) diff --git a/pddl/_validation.py b/pddl/_validation.py index 94fbe757..5aef0d8f 100644 --- a/pddl/_validation.py +++ b/pddl/_validation.py @@ -14,7 +14,7 @@ import functools from collections.abc import Iterable -from typing import AbstractSet, Collection, Dict, Optional, Set, Tuple, cast +from typing import AbstractSet, Collection, Dict, Optional, Set, Tuple, Type, cast from pddl.action import Action from pddl.custom_types import name as name_type @@ -186,6 +186,35 @@ def _check_types_dictionary( ) +def _check_names_not_a_keyword(names: Collection[name_type]) -> None: + """Check that items are not keywords.""" + _check_not_a_keyword(names, "name", ignore=set(), exception_cls=PDDLValidationError) + + +def _check_types_not_a_keyword(names: Collection[name_type]) -> None: + """ + Check that types are not keywords. + + We ignore 'object' as it is a proper type. + """ + _check_not_a_keyword( + names, "type", ignore={Symbols.OBJECT.value}, exception_cls=PDDLValidationError + ) + + +def _check_not_a_keyword( + names: Collection[name_type], + item_type: str, + ignore: Set[str], + exception_cls: Type[Exception] = ValueError, +) -> None: + """Check that the item name is not a keyword.""" + for item_name in names: + # we ignore 'object' as it is a proper type + if _is_a_keyword(item_name, ignore=ignore): + raise exception_cls(f"invalid {item_type} '{item_name}': it is a keyword") + + class TypeChecker: """Implementation of a type checker for domains and problems.""" @@ -232,6 +261,7 @@ def _(self, objects: Iterable) -> None: def _(self, term: Term) -> None: """Check types annotations of a PDDL term.""" self._check_typing_requirement(term.type_tags) + _check_types_not_a_keyword(term.type_tags) self._check_types_are_available(term.type_tags, f"term {repr(term)}") @check_type.register diff --git a/pddl/parser/types_index.py b/pddl/parser/types_index.py index 121eec0a..93697722 100644 --- a/pddl/parser/types_index.py +++ b/pddl/parser/types_index.py @@ -13,11 +13,11 @@ """Utility to handle typed lists.""" import itertools from collections import OrderedDict -from typing import Collection, Dict, List, Optional +from typing import Dict, List, Optional from typing import OrderedDict as OrderedDictType from typing import Set, Union -from pddl._validation import _is_a_keyword +from pddl._validation import _check_names_not_a_keyword, _check_types_not_a_keyword from pddl.custom_types import name from pddl.helpers.base import check, safe_index from pddl.parser.symbols import Symbols @@ -47,8 +47,8 @@ def add_item(self, item_name: name, type_tags: Set[name]) -> None: :param item_name: the item name :param type_tags: the types for the item """ - self._check_names_not_a_keyword({item_name}) - self._check_types_not_a_keyword(type_tags) + _check_names_not_a_keyword({item_name}) + _check_types_not_a_keyword(type_tags) self._check_item_name_already_present(item_name) self._check_tags_already_present(item_name, type_tags) self._add_item(item_name, type_tags) @@ -207,24 +207,3 @@ def _raise_multiple_types_error( f"typed list names should not have more than one type, got '{item_name}' with " f"types {sorted(map(str, types_tags))}" ) - - def _check_names_not_a_keyword(self, names: Collection[name]) -> None: - """Check that items are not keywords.""" - self._check_not_a_keyword(names, "name", ignore=set()) - - def _check_types_not_a_keyword(self, names: Collection[name]) -> None: - """ - Check that types are not keywords. - - We ignore 'object' as it is a proper type. - """ - self._check_not_a_keyword(names, "type", ignore={Symbols.OBJECT.value}) - - def _check_not_a_keyword( - self, names: Collection[name], item_type: str, ignore: Set[str] - ) -> None: - """Check that the item name is not a keyword.""" - for item_name in names: - # we ignore 'object' as it is a proper type - if _is_a_keyword(item_name, ignore=ignore): - raise ValueError(f"invalid {item_type} '{item_name}': it is a keyword") diff --git a/tests/test_parser/test_domain.py b/tests/test_parser/test_domain.py index 2475f4a3..e7ca0bdc 100644 --- a/tests/test_parser/test_domain.py +++ b/tests/test_parser/test_domain.py @@ -148,8 +148,7 @@ def test_keyword_usage_not_allowed_as_name(keyword) -> None: with pytest.raises( lark.exceptions.VisitError, - match=f".*error while parsing tokens \\['{keyword}'\\]: " - f"invalid name '{keyword}': it is a keyword", + match=f".*invalid name '{keyword}': it is a keyword", ): DomainParser()(domain_str) @@ -168,7 +167,6 @@ def test_keyword_usage_not_allowed_as_type(keyword) -> None: with pytest.raises( lark.exceptions.VisitError, - match=f".*error while parsing tokens \\['t1', '-', '{keyword}'\\]: " - f"invalid type '{keyword}': it is a keyword", + match=f".*invalid type '{keyword}': it is a keyword", ): DomainParser()(domain_str) From 7b3e7ad4f8b67434e939e867c44abbc2b4a658ab Mon Sep 17 00:00:00 2001 From: Marco Favorito Date: Wed, 7 Jun 2023 23:47:20 +0200 Subject: [PATCH 37/51] test: add tests on wrong variable typed lists --- pddl/parser/domain.py | 16 +++++--- tests/test_parser/test_domain.py | 63 ++++++++++++++++++++++++++------ 2 files changed, 61 insertions(+), 18 deletions(-) diff --git a/pddl/parser/domain.py b/pddl/parser/domain.py index 1efacb7d..8d365613 100644 --- a/pddl/parser/domain.py +++ b/pddl/parser/domain.py @@ -307,9 +307,7 @@ def typed_list_name(self, args) -> Dict[name, Optional[name]]: types_index = TypesIndex.parse_typed_list(args) return types_index.get_typed_list_of_names() except ValueError as e: - raise PDDLParsingError( - f"error while parsing tokens {list(map(str, args))}: {str(e)}" - ) from None + raise self._raise_typed_list_parsing_error(args, e) from e def typed_list_variable(self, args) -> OrderedDictType[name, Set[name]]: """ @@ -324,9 +322,15 @@ def typed_list_variable(self, args) -> OrderedDictType[name, Set[name]]: types_index = TypesIndex.parse_typed_list(args) return types_index.get_typed_list_of_variables() except ValueError as e: - raise PDDLParsingError( - f"error while parsing tokens {list(map(str, args))}: {str(e)}" - ) from None + raise self._raise_typed_list_parsing_error(args, e) from e + + def _raise_typed_list_parsing_error(self, args, exception) -> PDDLParsingError: + string_list = [ + str(arg) if isinstance(arg, str) else list(map(str, arg)) for arg in args + ] + return PDDLParsingError( + f"error while parsing tokens {string_list}: {str(exception)}" + ) def type_def(self, args): """Parse the 'type_def' rule.""" diff --git a/tests/test_parser/test_domain.py b/tests/test_parser/test_domain.py index e7ca0bdc..78ecb3e3 100644 --- a/tests/test_parser/test_domain.py +++ b/tests/test_parser/test_domain.py @@ -94,6 +94,44 @@ def test_types_repetition_in_typed_lists_not_allowed() -> None: DomainParser()(domain_str) +@pytest.mark.parametrize("keyword", TEXT_SYMBOLS - {Symbols.OBJECT.value}) +def test_keyword_usage_not_allowed_as_name(keyword) -> None: + """Check keywords usage as names is detected and a parsing error is raised.""" + domain_str = dedent( + f""" + (define (domain test) + (:requirements :typing) + (:types {keyword}) + ) + """ + ) + + with pytest.raises( + lark.exceptions.VisitError, + match=f".*invalid name '{keyword}': it is a keyword", + ): + DomainParser()(domain_str) + + +@pytest.mark.parametrize("keyword", TEXT_SYMBOLS - {Symbols.OBJECT.value}) +def test_keyword_usage_not_allowed_as_type(keyword) -> None: + """Check keywords usage as types is detected and a parsing error is raised.""" + domain_str = dedent( + f""" + (define (domain test) + (:requirements :typing) + (:types t1 - {keyword}) + ) + """ + ) + + with pytest.raises( + lark.exceptions.VisitError, + match=f".*invalid type '{keyword}': it is a keyword", + ): + DomainParser()(domain_str) + + def test_constants_repetition_in_simple_typed_lists_not_allowed() -> None: """Check constants repetition in simple typed lists is detected and a parsing error is raised.""" domain_str = dedent( @@ -134,39 +172,40 @@ def test_constants_repetition_in_typed_lists_not_allowed() -> None: DomainParser()(domain_str) -@pytest.mark.parametrize("keyword", TEXT_SYMBOLS - {Symbols.OBJECT.value}) -def test_keyword_usage_not_allowed_as_name(keyword) -> None: - """Check keywords usage as names is detected and a parsing error is raised.""" +def test_variables_repetition_in_simple_typed_lists_not_allowed() -> None: + """Check variables repetition in simple typed lists is detected and a parsing error is raised.""" domain_str = dedent( - f""" + """ (define (domain test) (:requirements :typing) - (:types {keyword}) + (:predicates (p ?x ?y ?z ?x)) ) """ ) with pytest.raises( lark.exceptions.VisitError, - match=f".*invalid name '{keyword}': it is a keyword", + match=".*error while parsing tokens \\['x', 'y', 'z', 'x'\\]: " + "duplicate name 'x' in typed list already present", ): DomainParser()(domain_str) -@pytest.mark.parametrize("keyword", TEXT_SYMBOLS - {Symbols.OBJECT.value}) -def test_keyword_usage_not_allowed_as_type(keyword) -> None: - """Check keywords usage as types is detected and a parsing error is raised.""" +def test_variables_repetition_in_typed_lists_not_allowed() -> None: + """Check variables repetition in typed lists is detected and a parsing error is raised.""" domain_str = dedent( - f""" + """ (define (domain test) (:requirements :typing) - (:types t1 - {keyword}) + (:types t1 t2) + (:predicates (p ?x - t1 ?x -t2)) ) """ ) with pytest.raises( lark.exceptions.VisitError, - match=f".*invalid type '{keyword}': it is a keyword", + match=r".*error while parsing tokens \['x', '-', \['t1'\], 'x', '-', \['t2'\]\]: duplicate name 'x' " + r"in typed list already present with types \['t1'\]", ): DomainParser()(domain_str) From a12e34bcdb3752b492c3c0225ae58b4aa653ec0f Mon Sep 17 00:00:00 2001 From: Marco Favorito Date: Thu, 8 Jun 2023 21:55:11 +0200 Subject: [PATCH 38/51] fix: move keyword validation inside class contructors This commit adds keyword validation for 'name' types. In particuar, instead of using the raw 'name' constructor, we added 'parse_name' and use it across modules. The same is done for 'types', except that the 'object' keyword should be ignored. This change is very helpful as the input validation is done during the initialization of the PDDL objects. The keyword validation functions have been moved to the pddl.custom_types module. --- pddl/_validation.py | 44 +++-------------------- pddl/action.py | 5 ++- pddl/core.py | 10 +++--- pddl/custom_types.py | 74 ++++++++++++++++++++++++++++++++++---- pddl/logic/predicates.py | 5 ++- pddl/logic/terms.py | 6 ++-- pddl/parser/types_index.py | 11 +++--- scripts/whitelist.py | 1 + tests/test_types.py | 28 ++++++++++++++- 9 files changed, 115 insertions(+), 69 deletions(-) diff --git a/pddl/_validation.py b/pddl/_validation.py index 5aef0d8f..32abe3f7 100644 --- a/pddl/_validation.py +++ b/pddl/_validation.py @@ -14,11 +14,11 @@ import functools from collections.abc import Iterable -from typing import AbstractSet, Collection, Dict, Optional, Set, Tuple, Type, cast +from typing import AbstractSet, Collection, Dict, Optional, Set, Tuple, cast from pddl.action import Action from pddl.custom_types import name as name_type -from pddl.custom_types import namelike, to_names, to_names_types # noqa: F401 +from pddl.custom_types import namelike, to_names, to_types # noqa: F401 from pddl.exceptions import PDDLValidationError from pddl.helpers.base import check, ensure, ensure_set, find_cycle from pddl.logic import Predicate @@ -32,7 +32,7 @@ from pddl.logic.effects import AndEffect, Forall, When from pddl.logic.predicates import DerivedPredicate, EqualTo from pddl.logic.terms import Term -from pddl.parser.symbols import ALL_SYMBOLS, Symbols +from pddl.parser.symbols import Symbols from pddl.requirements import Requirements @@ -82,12 +82,6 @@ def _check_types_in_has_terms_objects( ) -def _is_a_keyword(word: str, ignore: Optional[Set[str]] = None) -> bool: - """Check that the word is not a keyword.""" - ignore_set = ensure_set(ignore) - return word not in ignore_set and word in ALL_SYMBOLS - - class Types: """A class for representing and managing the types available in a PDDL Domain.""" @@ -98,7 +92,7 @@ def __init__( skip_checks: bool = False, ) -> None: """Initialize the Types object.""" - self._types = to_names_types(ensure(types, dict())) + self._types = to_types(ensure(types, dict())) self._all_types = self._get_all_types() @@ -186,35 +180,6 @@ def _check_types_dictionary( ) -def _check_names_not_a_keyword(names: Collection[name_type]) -> None: - """Check that items are not keywords.""" - _check_not_a_keyword(names, "name", ignore=set(), exception_cls=PDDLValidationError) - - -def _check_types_not_a_keyword(names: Collection[name_type]) -> None: - """ - Check that types are not keywords. - - We ignore 'object' as it is a proper type. - """ - _check_not_a_keyword( - names, "type", ignore={Symbols.OBJECT.value}, exception_cls=PDDLValidationError - ) - - -def _check_not_a_keyword( - names: Collection[name_type], - item_type: str, - ignore: Set[str], - exception_cls: Type[Exception] = ValueError, -) -> None: - """Check that the item name is not a keyword.""" - for item_name in names: - # we ignore 'object' as it is a proper type - if _is_a_keyword(item_name, ignore=ignore): - raise exception_cls(f"invalid {item_type} '{item_name}': it is a keyword") - - class TypeChecker: """Implementation of a type checker for domains and problems.""" @@ -261,7 +226,6 @@ def _(self, objects: Iterable) -> None: def _(self, term: Term) -> None: """Check types annotations of a PDDL term.""" self._check_typing_requirement(term.type_tags) - _check_types_not_a_keyword(term.type_tags) self._check_types_are_available(term.type_tags, f"term {repr(term)}") @check_type.register diff --git a/pddl/action.py b/pddl/action.py index 1db8f2b6..3407ca36 100644 --- a/pddl/action.py +++ b/pddl/action.py @@ -13,8 +13,7 @@ """This module contains the definition of the PDDL action class.""" from typing import Optional, Sequence -from pddl.custom_types import name as name_type -from pddl.custom_types import namelike +from pddl.custom_types import namelike, parse_name from pddl.helpers.base import _typed_parameters, ensure_sequence from pddl.logic import Variable from pddl.logic.base import Formula @@ -39,7 +38,7 @@ def __init__( :param precondition: the action precondition. :param effect: the action effect. """ - self._name: str = name_type(name) + self._name: str = parse_name(name) self._parameters: Sequence[Variable] = ensure_sequence(parameters) self._precondition = precondition self._effect = effect diff --git a/pddl/core.py b/pddl/core.py index 15839149..196d72b8 100644 --- a/pddl/core.py +++ b/pddl/core.py @@ -25,7 +25,7 @@ ) from pddl.action import Action from pddl.custom_types import name as name_type -from pddl.custom_types import namelike, to_names, to_names_types # noqa: F401 +from pddl.custom_types import namelike, parse_name, to_names, to_types # noqa: F401 from pddl.helpers.base import assert_, check, ensure, ensure_set from pddl.logic.base import Formula, TrueFormula, is_literal from pddl.logic.predicates import DerivedPredicate, Predicate @@ -60,7 +60,7 @@ def __init__( :param derived_predicates: the derived predicates. :param actions: the actions. """ - self._name = name_type(name) + self._name = parse_name(name) self._requirements = ensure_set(requirements) self._types = Types(types, self._requirements) self._constants = ensure_set(constants) @@ -161,7 +161,7 @@ def __init__( :param init: the initial condition. :param goal: the goal condition. """ - self._name = name_type(name) + self._name = parse_name(name) self._domain: Optional[Domain] self._domain_name: name_type self._domain, self._domain_name = self._parse_domain_and_domain_name( @@ -196,11 +196,11 @@ def _parse_domain_and_domain_name( f"got both domain and domain_name, but domain_name differs: {domain.name} != {domain_name}", exception_cls=ValueError, ) - return domain, name_type(domain_name) + return domain, parse_name(domain_name) if domain is not None: return domain, domain.name if domain_name is not None: - return None, name_type(domain_name) + return None, parse_name(domain_name) raise ValueError("Either domain or domain_name must be given.") def _parse_requirements( diff --git a/pddl/custom_types.py b/pddl/custom_types.py index 9d608558..9b4cbaf2 100644 --- a/pddl/custom_types.py +++ b/pddl/custom_types.py @@ -13,9 +13,11 @@ """This module defines useful custom types.""" import re -from typing import Collection, Dict, List, Optional, Union +from typing import AbstractSet, Collection, Dict, List, Optional, Union -from pddl.helpers.base import RegexConstrainedString +from pddl.exceptions import PDDLValidationError +from pddl.helpers.base import RegexConstrainedString, ensure_set +from pddl.parser.symbols import ALL_SYMBOLS, Symbols class name(RegexConstrainedString): @@ -36,20 +38,78 @@ class name(RegexConstrainedString): this can be achieved thanks to 'name' constructor idempotency, without explicitly caring of whether the arguments are actually a 'name' or a 'str'. + +Note, you should not use the raw constructor 'name' (unless you +know what you are doing), but the functino 'parse_name' below. """ namelike = Union[name, str] +def parse_name(s: str) -> name: + """ + Parse a name from a string. + + It performs two validations: + - the name is not a keyword; + - the name is a valid name, i.e. it matches the name regular expression. + + Optionally, a set of keywords to ignore can be provided. + + :param s: the input string to be parsed + :return: the parsed name + """ + _check_not_a_keyword(s, "name", ignore=set()) + return name(s) + + +def parse_type(s: str) -> name: + """ + Parse a type from a string. + + It performs two validations: + - the type is not a keyword; + - the type is a valid name, i.e. it matches the name regular expression. + + The type name 'object' is allowed. + + :param s: the input string to be parsed + :return: the parsed type name + """ + _check_not_a_keyword(s, "type", ignore={Symbols.OBJECT.value}) + return name(s) + + def to_names(names: Collection[namelike]) -> List[name]: """From name-like sequence to list of names.""" - return list(map(name, names)) + return list(map(parse_name, names)) + +def to_type(names: Collection[namelike]) -> List[name]: + """From name-like sequence to list of type names.""" + return list(map(parse_type, names)) -def to_names_types( - names: Dict[namelike, Optional[namelike]] -) -> Dict[name, Optional[name]]: + +def to_types(names: Dict[namelike, Optional[namelike]]) -> Dict[name, Optional[name]]: """From name-like dictionary to name dictionary.""" return { - name(type_): name(ancestor) if ancestor else None + parse_type(type_): parse_type(ancestor) if ancestor else None for type_, ancestor in names.items() } + + +def _is_a_keyword(word: str, ignore: Optional[AbstractSet[str]] = None) -> bool: + """Check that the word is not a keyword.""" + ignore_set = ensure_set(ignore) + return word not in ignore_set and word in ALL_SYMBOLS + + +def _check_not_a_keyword( + input_name: str, + item_type: str, + ignore: AbstractSet[str], +) -> None: + """Check that the item name is not a keyword.""" + if _is_a_keyword(input_name, ignore=ignore): + raise PDDLValidationError( + f"invalid {item_type} '{input_name}': it is a keyword" + ) diff --git a/pddl/logic/predicates.py b/pddl/logic/predicates.py index cc9b9944..f044c942 100644 --- a/pddl/logic/predicates.py +++ b/pddl/logic/predicates.py @@ -14,8 +14,7 @@ import functools from typing import Sequence -from pddl.custom_types import name as name_type -from pddl.custom_types import namelike +from pddl.custom_types import namelike, parse_name from pddl.helpers.base import assert_ from pddl.helpers.cache_hash import cache_hash from pddl.logic.base import Atomic, Formula @@ -30,7 +29,7 @@ class Predicate(Atomic): def __init__(self, name: namelike, *terms: Term): """Initialize the predicate.""" - self._name = name_type(name) + self._name = parse_name(name) self._terms = tuple(terms) @property diff --git a/pddl/logic/terms.py b/pddl/logic/terms.py index 0f69eb08..f340b9d9 100644 --- a/pddl/logic/terms.py +++ b/pddl/logic/terms.py @@ -15,7 +15,7 @@ from typing import AbstractSet, Collection, Optional from pddl.custom_types import name as name_type -from pddl.custom_types import namelike, to_names +from pddl.custom_types import namelike, parse_name, to_type from pddl.helpers.base import assert_, ensure_set from pddl.helpers.cache_hash import cache_hash @@ -34,8 +34,8 @@ def __init__( :param name: the name for the term. :param type_tags: the type tags associated to this term. """ - self._name = name_type(name) - self._type_tags = set(to_names(ensure_set(type_tags))) + self._name = parse_name(name) + self._type_tags = frozenset(to_type(ensure_set(type_tags))) @property def name(self) -> str: diff --git a/pddl/parser/types_index.py b/pddl/parser/types_index.py index 93697722..4028230a 100644 --- a/pddl/parser/types_index.py +++ b/pddl/parser/types_index.py @@ -15,10 +15,9 @@ from collections import OrderedDict from typing import Dict, List, Optional from typing import OrderedDict as OrderedDictType -from typing import Set, Union +from typing import Set, Union, cast -from pddl._validation import _check_names_not_a_keyword, _check_types_not_a_keyword -from pddl.custom_types import name +from pddl.custom_types import name, parse_name, parse_type from pddl.helpers.base import check, safe_index from pddl.parser.symbols import Symbols @@ -47,8 +46,6 @@ def add_item(self, item_name: name, type_tags: Set[name]) -> None: :param item_name: the item name :param type_tags: the types for the item """ - _check_names_not_a_keyword({item_name}) - _check_types_not_a_keyword(type_tags) self._check_item_name_already_present(item_name) self._check_tags_already_present(item_name, type_tags) self._add_item(item_name, type_tags) @@ -159,8 +156,8 @@ def _add_typed_lists( isinstance(item_name, str), f"invalid item '{item_name}' in typed list" ) # these lines implicitly perform name validation - item_name = name(item_name) - type_tags_names: Set[name] = set(map(name, type_tags)) + item_name = parse_name(cast(str, item_name)) + type_tags_names: Set[name] = set(map(parse_type, type_tags)) result.add_item(item_name, type_tags_names) def _check_item_name_already_present(self, item_name: name) -> None: diff --git a/scripts/whitelist.py b/scripts/whitelist.py index 746d7e36..5477af74 100644 --- a/scripts/whitelist.py +++ b/scripts/whitelist.py @@ -48,3 +48,4 @@ REQUIREMENTS # unused variable (pddl/parser/symbols.py:51) TYPES # unused variable (pddl/parser/symbols.py:52) ALL_REQUIREMENTS # unused variable (pddl/parser/symbols.py:80) +to_names # unused function (pddl/custom_types.py:79) diff --git a/tests/test_types.py b/tests/test_types.py index e66cd7f5..d2ed8877 100644 --- a/tests/test_types.py +++ b/tests/test_types.py @@ -14,7 +14,10 @@ import pytest -from pddl.custom_types import name +from pddl.custom_types import name, parse_name, parse_type +from pddl.exceptions import PDDLValidationError +from pddl.parser.symbols import Symbols +from tests.conftest import TEXT_SYMBOLS def test_name_string(): @@ -41,3 +44,26 @@ def test_name_starts_with_digits(): """Test that providing a string to name constructor starting with digits raises error.""" with pytest.raises(ValueError): name("123") + + +@pytest.mark.parametrize("keyword", TEXT_SYMBOLS) +def test_name_is_a_keyword(keyword): + """Test that parse_name with keywords as input raises error.""" + with pytest.raises( + PDDLValidationError, match=f"invalid name '{keyword}': it is a keyword" + ): + parse_name(keyword) + + +@pytest.mark.parametrize("keyword", TEXT_SYMBOLS - {Symbols.OBJECT.value}) +def test_type_is_a_keyword(keyword): + """Test that parse_type with keywords as input raises error.""" + with pytest.raises( + PDDLValidationError, match=f"invalid type '{keyword}': it is a keyword" + ): + parse_type(keyword) + + +def test_object_is_a_valid_type_name(): + """Test that parse_type with input 'object' does not raise error.""" + parse_type(Symbols.OBJECT.value) From f0e79afafa77263f44534a0f31c50ce628c53d3b Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Fri, 9 Jun 2023 10:24:50 +0200 Subject: [PATCH 39/51] fix: make find_cycle to handle arbitrary graphs --- pddl/_validation.py | 8 +++++++- pddl/helpers/base.py | 15 ++++++--------- tests/test_helpers/test_base.py | 4 ++-- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/pddl/_validation.py b/pddl/_validation.py index 32abe3f7..4f281bc2 100644 --- a/pddl/_validation.py +++ b/pddl/_validation.py @@ -173,7 +173,13 @@ def _check_types_dictionary( ) # check cycles - cycle = find_cycle(type_dict) # type: ignore + # need to convert type_dict to a dict of sets, because find_cycle() expects a dict of sets + cycle = find_cycle( + { + key: {value} if value is not None else set() + for key, value in type_dict.items() + } + ) # type: ignore if cycle is not None: raise PDDLValidationError( "cycle detected in the type hierarchy: " + " -> ".join(cycle) diff --git a/pddl/helpers/base.py b/pddl/helpers/base.py index fe0806ac..e4b0bfd0 100644 --- a/pddl/helpers/base.py +++ b/pddl/helpers/base.py @@ -152,12 +152,8 @@ def _handle_no_match(self): ) -def find_cycle(graph: Dict[str, Optional[str]]) -> Optional[Sequence[str]]: - """ - Check whether a graph (represented as a dictionary-based adjacency list) has a cycle. - - This implementation assumes the constraint that each node has at most one successor. - """ +def find_cycle(graph: Dict[str, Optional[AbstractSet[str]]]) -> Optional[Sequence[str]]: + """Check whether a graph (represented as a dictionary-based adjacency list) has a cycle.""" visited: Set = set() stack: List = [] @@ -171,8 +167,9 @@ def find_cycle(graph: Dict[str, Optional[str]]) -> Optional[Sequence[str]]: return path visited.add(current) - neighbor = graph.get(current) - if neighbor is not None: - stack.append((neighbor, path + [current])) + neighbors = graph.get(current) + if neighbors is not None: + for neighbor in neighbors: + stack.append((neighbor, path + [current])) return None diff --git a/tests/test_helpers/test_base.py b/tests/test_helpers/test_base.py index 083682e2..ce9f9f21 100644 --- a/tests/test_helpers/test_base.py +++ b/tests/test_helpers/test_base.py @@ -22,10 +22,10 @@ def test_find_cycle_empty_graph() -> None: def test_find_cycle_negative_case() -> None: """Test 'find_cycle' function, negative case.""" a, b, c, d = "a b c d".split() - assert find_cycle({a: b, b: c, d: c}) is None + assert find_cycle({a: {b}, b: {c}, d: {c}}) is None def test_find_cycle_positive_case() -> None: """Test 'find_cycle' function, positive case.""" a, b, c, d = "a b c d".split() - assert find_cycle({a: b, b: c, d: c, c: a}) == ["a", "b", "c"] + assert find_cycle({a: {b}, b: {c}, d: {c}, c: {a}}) == ["a", "b", "c"] From 8c418308afba8b271b73197a417644b6104ec7a8 Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Fri, 9 Jun 2023 11:33:22 +0200 Subject: [PATCH 40/51] chore: rename TypesIndex to TypedListParser --- pddl/parser/domain.py | 6 +++--- .../{types_index.py => typed_list_parser.py} | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) rename pddl/parser/{types_index.py => typed_list_parser.py} (95%) diff --git a/pddl/parser/domain.py b/pddl/parser/domain.py index 8d365613..682fa423 100644 --- a/pddl/parser/domain.py +++ b/pddl/parser/domain.py @@ -38,7 +38,7 @@ from pddl.logic.terms import Constant, Variable from pddl.parser import DOMAIN_GRAMMAR_FILE, PARSERS_DIRECTORY from pddl.parser.symbols import Symbols -from pddl.parser.types_index import TypesIndex +from pddl.parser.typed_list_parser import TypedListParser from pddl.requirements import Requirements @@ -304,7 +304,7 @@ def atomic_formula_skeleton(self, args): def typed_list_name(self, args) -> Dict[name, Optional[name]]: """Process the 'typed_list_name' rule.""" try: - types_index = TypesIndex.parse_typed_list(args) + types_index = TypedListParser.parse_typed_list(args) return types_index.get_typed_list_of_names() except ValueError as e: raise self._raise_typed_list_parsing_error(args, e) from e @@ -319,7 +319,7 @@ def typed_list_variable(self, args) -> OrderedDictType[name, Set[name]]: :return: a typed list (variable), i.e. a mapping from variables to the supported types """ try: - types_index = TypesIndex.parse_typed_list(args) + types_index = TypedListParser.parse_typed_list(args) return types_index.get_typed_list_of_variables() except ValueError as e: raise self._raise_typed_list_parsing_error(args, e) from e diff --git a/pddl/parser/types_index.py b/pddl/parser/typed_list_parser.py similarity index 95% rename from pddl/parser/types_index.py rename to pddl/parser/typed_list_parser.py index 4028230a..4e8d8378 100644 --- a/pddl/parser/types_index.py +++ b/pddl/parser/typed_list_parser.py @@ -22,7 +22,7 @@ from pddl.parser.symbols import Symbols -class TypesIndex: +class TypedListParser: """ An index for PDDL types and PDDL names/variables. @@ -65,7 +65,7 @@ def get_typed_list_of_variables(self) -> OrderedDictType[name, Set[name]]: return self._item_to_types @classmethod - def parse_typed_list(cls, tokens: List[Union[str, List[str]]]) -> "TypesIndex": + def parse_typed_list(cls, tokens: List[Union[str, List[str]]]) -> "TypedListParser": """ Parse typed list. @@ -76,15 +76,15 @@ def parse_typed_list(cls, tokens: List[Union[str, List[str]]]) -> "TypesIndex": - if the list is not typed, it is simply a list of names - if the list is typed, the format is: [name_1, ..., name_n], "-", [type_1, ..., type_m], ... - >>> index = TypesIndex.parse_typed_list(["a", "b", "c"]) + >>> index = TypedListParser.parse_typed_list(["a", "b", "c"]) >>> index.get_typed_list_of_names() {'a': None, 'b': None, 'c': None} - >>> index = TypesIndex.parse_typed_list(["a", "b", "c", "-", ["t1"]]) + >>> index = TypedListParser.parse_typed_list(["a", "b", "c", "-", ["t1"]]) >>> index.get_typed_list_of_names() {'a': 't1', 'b': 't1', 'c': 't1'} - >>> index = TypesIndex.parse_typed_list(["a", "b", "c", "-", ["t1", "t2"]]) + >>> index = TypedListParser.parse_typed_list(["a", "b", "c", "-", ["t1", "t2"]]) >>> index.get_typed_list_of_names() Traceback (most recent call last): ... @@ -93,7 +93,7 @@ def parse_typed_list(cls, tokens: List[Union[str, List[str]]]) -> "TypesIndex": :param tokens: the list of tokens :return: the TypesIndex object """ - result = TypesIndex() + result = TypedListParser() type_sep_index = safe_index(tokens, Symbols.TYPE_SEP.value) @@ -139,7 +139,7 @@ def parse_typed_list(cls, tokens: List[Union[str, List[str]]]) -> "TypesIndex": @classmethod def _add_typed_lists( cls, - result: "TypesIndex", + result: "TypedListParser", start_index: int, end_index: int, tokens: List[Union[str, List[str]]], From eb2429d1be0da71a372a964635a991d75e93f3ea Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Fri, 9 Jun 2023 14:46:41 +0200 Subject: [PATCH 41/51] feat: allow repetition in variable list Although not very common, a list of variables might contain duplicated entries. For example, this allows to define a predicate like P(?x, ?x). --- pddl/parser/domain.py | 18 ++++------- pddl/parser/typed_list_parser.py | 55 ++++++++++++++++++++++---------- tests/test_parser/test_domain.py | 48 ++++++++++++++++++++++------ 3 files changed, 84 insertions(+), 37 deletions(-) diff --git a/pddl/parser/domain.py b/pddl/parser/domain.py index 682fa423..2e7ebd1f 100644 --- a/pddl/parser/domain.py +++ b/pddl/parser/domain.py @@ -12,9 +12,7 @@ """Implementation of the PDDL domain parser.""" import sys -from typing import Dict, Optional -from typing import OrderedDict as OrderedDictType -from typing import Set +from typing import Dict, Optional, Set, Tuple from lark import Lark, ParseError, Transformer @@ -133,7 +131,7 @@ def derived_predicates(self, args): def action_parameters(self, args): """Process the 'action_parameters' rule.""" self._current_parameters_by_name = { - var_name: Variable(var_name, tags) for var_name, tags in args[1].items() + var_name: Variable(var_name, tags) for var_name, tags in args[1] } return list(self._current_parameters_by_name.values()) @@ -197,7 +195,7 @@ def gd_quantifiers(self, args): & self._extended_requirements ): raise PDDLMissingRequirementError(req) - variables = [Variable(var_name, tags) for var_name, tags in args[3].items()] + variables = [Variable(var_name, tags) for var_name, tags in args[3]] condition = args[5] return cond_class(cond=condition, variables=variables) @@ -236,7 +234,7 @@ def c_effect(self, args): if len(args) == 1: return args[0] if args[1] == Symbols.FORALL.value: - variables = [Variable(var_name, tags) for var_name, tags in args[3].items()] + variables = [Variable(var_name, tags) for var_name, tags in args[3]] return Forall(effect=args[-2], variables=variables) if args[1] == Symbols.WHEN.value: return When(args[2], args[3]) @@ -296,9 +294,7 @@ def atomic_formula_skeleton(self, args): """Process the 'atomic_formula_skeleton' rule.""" predicate_name = args[1] variable_data: Dict[str, Set[str]] = args[2] - variables = [ - Variable(var_name, tags) for var_name, tags in variable_data.items() - ] + variables = [Variable(var_name, tags) for var_name, tags in variable_data] return Predicate(predicate_name, *variables) def typed_list_name(self, args) -> Dict[name, Optional[name]]: @@ -309,7 +305,7 @@ def typed_list_name(self, args) -> Dict[name, Optional[name]]: except ValueError as e: raise self._raise_typed_list_parsing_error(args, e) from e - def typed_list_variable(self, args) -> OrderedDictType[name, Set[name]]: + def typed_list_variable(self, args) -> Tuple[Tuple[name, Set[name]], ...]: """ Process the 'typed_list_variable' rule. @@ -319,7 +315,7 @@ def typed_list_variable(self, args) -> OrderedDictType[name, Set[name]]: :return: a typed list (variable), i.e. a mapping from variables to the supported types """ try: - types_index = TypedListParser.parse_typed_list(args) + types_index = TypedListParser.parse_typed_list(args, allow_duplicates=True) return types_index.get_typed_list_of_variables() except ValueError as e: raise self._raise_typed_list_parsing_error(args, e) from e diff --git a/pddl/parser/typed_list_parser.py b/pddl/parser/typed_list_parser.py index 4e8d8378..1bfc6575 100644 --- a/pddl/parser/typed_list_parser.py +++ b/pddl/parser/typed_list_parser.py @@ -12,10 +12,7 @@ """Utility to handle typed lists.""" import itertools -from collections import OrderedDict -from typing import Dict, List, Optional -from typing import OrderedDict as OrderedDictType -from typing import Set, Union, cast +from typing import Dict, List, Optional, Set, Tuple, Union, cast from pddl.custom_types import name, parse_name, parse_type from pddl.helpers.base import check, safe_index @@ -32,10 +29,14 @@ class TypedListParser: Other types of validations are performed to ensure that the types index is consistent. """ - def __init__(self) -> None: + def __init__(self, allow_duplicates: bool) -> None: """Initialize the types index.""" - self._types_to_items: OrderedDictType[name, Set[name]] = OrderedDict() - self._item_to_types: OrderedDictType[name, Set[name]] = OrderedDict() + self._allow_duplicates = allow_duplicates + + self._types_to_items: Dict[name, Set[name]] = {} + self._item_to_types: Dict[name, Set[name]] = {} + + self._item_to_types_sequence: List[Tuple[name, Set[name]]] = [] def add_item(self, item_name: name, type_tags: Set[name]) -> None: """ @@ -46,8 +47,10 @@ def add_item(self, item_name: name, type_tags: Set[name]) -> None: :param item_name: the item name :param type_tags: the types for the item """ - self._check_item_name_already_present(item_name) - self._check_tags_already_present(item_name, type_tags) + if not self._allow_duplicates: + self._check_item_name_already_present(item_name) + self._check_tags_already_present(item_name, type_tags) + self._check_item_types(item_name, type_tags) self._add_item(item_name, type_tags) def get_typed_list_of_names(self) -> Dict[name, Optional[name]]: @@ -60,12 +63,14 @@ def get_typed_list_of_names(self) -> Dict[name, Optional[name]]: result[item] = type_tag return result - def get_typed_list_of_variables(self) -> OrderedDictType[name, Set[name]]: - """Get the typed list of variables in form of dictionary.""" - return self._item_to_types + def get_typed_list_of_variables(self) -> Tuple[Tuple[name, Set[name]], ...]: + """Get the typed list of variables in form of a tuple of pairs.""" + return tuple(self._item_to_types_sequence) @classmethod - def parse_typed_list(cls, tokens: List[Union[str, List[str]]]) -> "TypedListParser": + def parse_typed_list( + cls, tokens: List[Union[str, List[str]]], allow_duplicates: bool = False + ) -> "TypedListParser": """ Parse typed list. @@ -91,9 +96,10 @@ def parse_typed_list(cls, tokens: List[Union[str, List[str]]]) -> "TypedListPars ValueError: typed list names should not have more than one type, got 'a' with types ['t1', 't2'] :param tokens: the list of tokens + :param allow_duplicates: whether allowed_duplicates are allowed :return: the TypesIndex object """ - result = TypedListParser() + result = TypedListParser(allow_duplicates=allow_duplicates) type_sep_index = safe_index(tokens, Symbols.TYPE_SEP.value) @@ -190,17 +196,34 @@ def _check_tags_already_present( f"duplicate type tag '{type_tag}' in typed list: type already specified for item {item_name}" ) + def _check_item_types(self, item_name: name, type_tags: Set[name]) -> None: + """Check if the types of the item are valid.""" + if item_name in self._item_to_types: + previous_type_tags = self._item_to_types[item_name] + if previous_type_tags != type_tags: + raise ValueError( + f"invalid types for item '{item_name}': previous known tags were " + f"{self._print_tag_set(previous_type_tags)}, got {self._print_tag_set(type_tags)}" + ) + def _add_item(self, item_name: name, type_tags: Set[name]) -> None: """Add an item (no validation).""" for type_tag in type_tags: self._types_to_items.setdefault(type_tag, set()).add(item_name) self._item_to_types.setdefault(item_name, set()).update(type_tags) + self._item_to_types_sequence.append((item_name, type_tags)) def _raise_multiple_types_error( - self, item_name: name, types_tags: Set[name] + self, item_name: name, type_tags: Set[name] ) -> None: """Raise an error if the item has multiple types.""" raise ValueError( f"typed list names should not have more than one type, got '{item_name}' with " - f"types {sorted(map(str, types_tags))}" + f"types {self._print_tag_set(type_tags)}" ) + + def _print_tag_set(self, type_tags: Set[name]) -> str: + """Print a tag set.""" + if len(type_tags) == 0: + return "[]" + return repr(sorted(map(str, type_tags))) diff --git a/tests/test_parser/test_domain.py b/tests/test_parser/test_domain.py index 78ecb3e3..b8244dc4 100644 --- a/tests/test_parser/test_domain.py +++ b/tests/test_parser/test_domain.py @@ -172,8 +172,8 @@ def test_constants_repetition_in_typed_lists_not_allowed() -> None: DomainParser()(domain_str) -def test_variables_repetition_in_simple_typed_lists_not_allowed() -> None: - """Check variables repetition in simple typed lists is detected and a parsing error is raised.""" +def test_variables_repetition_in_simple_typed_lists_allowed() -> None: + """Check variables repetition in simple typed lists is allowed.""" domain_str = dedent( """ (define (domain test) @@ -183,29 +183,57 @@ def test_variables_repetition_in_simple_typed_lists_not_allowed() -> None: """ ) + DomainParser()(domain_str) + + +def test_variables_repetition_in_typed_lists_not_allowed() -> None: + """Check variables repetition in typed lists is detected and a parsing error is raised.""" + domain_str = dedent( + """ + (define (domain test) + (:requirements :typing) + (:types t1 t2) + (:predicates (p ?x - (either t1 t2) ?x - t3)) + ) + """ + ) + with pytest.raises( lark.exceptions.VisitError, - match=".*error while parsing tokens \\['x', 'y', 'z', 'x'\\]: " - "duplicate name 'x' in typed list already present", + match=r".*error while parsing tokens \['x', '-', \['t1', 't2'\], 'x', '-', \['t3'\]\]: " + r"invalid types for item \'x\': previous known tags were \['t1', 't2'\], got \['t3'\]", ): DomainParser()(domain_str) -def test_variables_repetition_in_typed_lists_not_allowed() -> None: - """Check variables repetition in typed lists is detected and a parsing error is raised.""" +def test_variables_typed_with_not_available_types() -> None: + """Check variables with non-available types raises a parsing error.""" domain_str = dedent( """ (define (domain test) (:requirements :typing) - (:types t1 t2) - (:predicates (p ?x - t1 ?x -t2)) + (:types t1) + (:predicates (p ?x - t2)) ) """ ) with pytest.raises( lark.exceptions.VisitError, - match=r".*error while parsing tokens \['x', '-', \['t1'\], 'x', '-', \['t2'\]\]: duplicate name 'x' " - r"in typed list already present with types \['t1'\]", + match=r"types \['t2'\] of term Variable\(x\) are not in available types \{'t1'\}", ): DomainParser()(domain_str) + + +def test_variables_repetition_allowed_if_same_type() -> None: + """Check variables repetition in typed lists is detected and a parsing error is raised.""" + domain_str = dedent( + """ + (define (domain test) + (:requirements :typing) + (:types t1) + (:predicates (p ?x - t1 ?x - t1)) + ) + """ + ) + DomainParser()(domain_str) From 9a168587e94c024ce97dfedc1a359d6cd55d267e Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Fri, 9 Jun 2023 15:03:09 +0200 Subject: [PATCH 42/51] test: improve 'test_variables_repetition_allowed_if_same_type' add tests on multi-types, predicates, action parameters/preconditions/effects. --- tests/test_parser/test_domain.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/test_parser/test_domain.py b/tests/test_parser/test_domain.py index b8244dc4..d4e2cb19 100644 --- a/tests/test_parser/test_domain.py +++ b/tests/test_parser/test_domain.py @@ -226,13 +226,18 @@ def test_variables_typed_with_not_available_types() -> None: def test_variables_repetition_allowed_if_same_type() -> None: - """Check variables repetition in typed lists is detected and a parsing error is raised.""" + """Check variables repetition in typed lists is allowed.""" domain_str = dedent( """ (define (domain test) - (:requirements :typing) - (:types t1) - (:predicates (p ?x - t1 ?x - t1)) + (:requirements :typing :existential-preconditions :universal-preconditions) + (:types t1 t2) + (:predicates (p ?x - (either t1 t2) ?x - (either t1 t2))) + (:action a + :parameters (?x - t1 ?x - t1) + :precondition (and (exists (?x - t1) (p ?x ?x)) (forall (?x - t1) (p ?x ?x))) + :effect (p ?x ?x) + ) ) """ ) From a6367ba08dab755aeb168ea38cac7d5c6648dc97 Mon Sep 17 00:00:00 2001 From: Marco Favorito Date: Fri, 9 Jun 2023 15:14:47 +0200 Subject: [PATCH 43/51] Update .github/workflows/docs.yml from @francescofuggitti PR comment Co-authored-by: Francesco Fuggitti --- .github/workflows/docs.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 5efd2f91..2c3a8b50 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -13,7 +13,7 @@ jobs: strategy: matrix: os: [ubuntu-latest] - python-version: [3.8] + python-version: ["3.8"] timeout-minutes: 30 From 2c67fde8afc19759e626fa3b8696bfe1921c5eab Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Fri, 9 Jun 2023 15:35:22 +0200 Subject: [PATCH 44/51] feat: add validation of no-duplicates in type-tags of a Term --- pddl/helpers/base.py | 16 ++++++++++++++++ pddl/logic/terms.py | 4 ++-- tests/test_logic/__init__.py | 13 +++++++++++++ tests/test_logic/test_terms.py | 24 ++++++++++++++++++++++++ 4 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 tests/test_logic/__init__.py create mode 100644 tests/test_logic/test_terms.py diff --git a/pddl/helpers/base.py b/pddl/helpers/base.py index e4b0bfd0..dcc30dc1 100644 --- a/pddl/helpers/base.py +++ b/pddl/helpers/base.py @@ -72,6 +72,22 @@ def ensure_set(arg: Optional[Collection], immutable: bool = True) -> AbstractSet return op(arg) if arg is not None else op() +def check_no_duplicates(arg: Optional[Sequence[str]]) -> Optional[Collection]: + """Check that the argument is a set.""" + if arg is None: + return None + if isinstance(arg, AbstractSet): + return arg + seen = set() + for x in arg: + if x in seen: + raise ValueError( + f"duplicate element in collection {list(map(str, arg))}: '{str(x)}'" + ) + seen.add(x) + return arg + + def ensure_sequence(arg: Optional[Sequence], immutable: bool = True) -> Sequence: """ Ensure the argument is a sequence. diff --git a/pddl/logic/terms.py b/pddl/logic/terms.py index f340b9d9..86412337 100644 --- a/pddl/logic/terms.py +++ b/pddl/logic/terms.py @@ -16,7 +16,7 @@ from pddl.custom_types import name as name_type from pddl.custom_types import namelike, parse_name, to_type -from pddl.helpers.base import assert_, ensure_set +from pddl.helpers.base import assert_, check_no_duplicates, ensure_set from pddl.helpers.cache_hash import cache_hash @@ -35,7 +35,7 @@ def __init__( :param type_tags: the type tags associated to this term. """ self._name = parse_name(name) - self._type_tags = frozenset(to_type(ensure_set(type_tags))) + self._type_tags = frozenset(to_type(ensure_set(check_no_duplicates(type_tags)))) # type: ignore @property def name(self) -> str: diff --git a/tests/test_logic/__init__.py b/tests/test_logic/__init__.py new file mode 100644 index 00000000..616557ad --- /dev/null +++ b/tests/test_logic/__init__.py @@ -0,0 +1,13 @@ +# +# Copyright 2021-2023 WhiteMech +# +# ------------------------------ +# +# This file is part of pddl. +# +# Use of this source code is governed by an MIT-style +# license that can be found in the LICENSE file or at +# https://opensource.org/licenses/MIT. +# + +"""Test pddl.logic module.""" diff --git a/tests/test_logic/test_terms.py b/tests/test_logic/test_terms.py new file mode 100644 index 00000000..0b214c38 --- /dev/null +++ b/tests/test_logic/test_terms.py @@ -0,0 +1,24 @@ +# +# Copyright 2021-2023 WhiteMech +# +# ------------------------------ +# +# This file is part of pddl. +# +# Use of this source code is governed by an MIT-style +# license that can be found in the LICENSE file or at +# https://opensource.org/licenses/MIT. +# + +"""Test pddl.logic module.""" +import pytest + +from pddl.logic.terms import Term + + +def test_no_duplicated_type_tags() -> None: + """Test that no duplicated type tags are allowed.""" + with pytest.raises( + ValueError, match=r"duplicate element in collection \['b', 'b'\]: 'b'" + ): + Term("a", ["b", "b"]) From b8e0a2b0fe4a4a35d10fcd608b4ed8ea9ca45766 Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Fri, 9 Jun 2023 16:31:46 +0200 Subject: [PATCH 45/51] feat: make Term non-instantiatable --- pddl/logic/terms.py | 1 + tests/test_logic/test_terms.py | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/pddl/logic/terms.py b/pddl/logic/terms.py index 86412337..22e578f4 100644 --- a/pddl/logic/terms.py +++ b/pddl/logic/terms.py @@ -34,6 +34,7 @@ def __init__( :param name: the name for the term. :param type_tags: the type tags associated to this term. """ + assert_(type(self) is not Term, "Term is an abstract class") self._name = parse_name(name) self._type_tags = frozenset(to_type(ensure_set(check_no_duplicates(type_tags)))) # type: ignore diff --git a/tests/test_logic/test_terms.py b/tests/test_logic/test_terms.py index 0b214c38..70163327 100644 --- a/tests/test_logic/test_terms.py +++ b/tests/test_logic/test_terms.py @@ -13,7 +13,7 @@ """Test pddl.logic module.""" import pytest -from pddl.logic.terms import Term +from pddl.logic.terms import Variable def test_no_duplicated_type_tags() -> None: @@ -21,4 +21,4 @@ def test_no_duplicated_type_tags() -> None: with pytest.raises( ValueError, match=r"duplicate element in collection \['b', 'b'\]: 'b'" ): - Term("a", ["b", "b"]) + Variable("a", ["b", "b"]) From 3661f64e946130208bedc59311731afd349878b9 Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Fri, 9 Jun 2023 16:20:49 +0200 Subject: [PATCH 46/51] feat: check terms consistency wrt type tag i.e. terms with the same name should have the same type tags. --- pddl/helpers/base.py | 2 +- pddl/logic/predicates.py | 32 +++++++++++++++++++++++------ pddl/logic/terms.py | 15 ++++++++++---- pddl/parser/typed_list_parser.py | 11 +++------- tests/test_logic/test_predicates.py | 28 +++++++++++++++++++++++++ tests/test_logic/test_terms.py | 2 +- 6 files changed, 70 insertions(+), 20 deletions(-) create mode 100644 tests/test_logic/test_predicates.py diff --git a/pddl/helpers/base.py b/pddl/helpers/base.py index dcc30dc1..ad5b89b1 100644 --- a/pddl/helpers/base.py +++ b/pddl/helpers/base.py @@ -72,7 +72,7 @@ def ensure_set(arg: Optional[Collection], immutable: bool = True) -> AbstractSet return op(arg) if arg is not None else op() -def check_no_duplicates(arg: Optional[Sequence[str]]) -> Optional[Collection]: +def check_no_duplicates(arg: Optional[Collection]) -> Optional[Collection]: """Check that the argument is a set.""" if arg is None: return None diff --git a/pddl/logic/predicates.py b/pddl/logic/predicates.py index f044c942..615bfc8d 100644 --- a/pddl/logic/predicates.py +++ b/pddl/logic/predicates.py @@ -12,25 +12,45 @@ """This class implements PDDL predicates.""" import functools -from typing import Sequence +from typing import Dict, Sequence, Set -from pddl.custom_types import namelike, parse_name -from pddl.helpers.base import assert_ +from pddl.custom_types import name, namelike, parse_name +from pddl.helpers.base import assert_, check from pddl.helpers.cache_hash import cache_hash from pddl.logic.base import Atomic, Formula -from pddl.logic.terms import Term +from pddl.logic.terms import Term, _print_tag_set from pddl.parser.symbols import Symbols +def _check_terms_consistency(terms: Sequence[Term]): + """ + Check that the term sequence have consistent type tags. + + In particular, terms with the same name must have the same type tags. + """ + seen: Dict[name, Set[name]] = {} + for term in terms: + if term.name not in seen: + seen[term.name] = set(term.type_tags) + else: + check( + seen[term.name] == set(term.type_tags), + f"Term {term} has inconsistent type tags: " + f"previous type tags {_print_tag_set(seen[term.name])}, new type tags {_print_tag_set(term.type_tags)}", + exception_cls=ValueError, + ) + + @cache_hash @functools.total_ordering class Predicate(Atomic): """A class for a Predicate in PDDL.""" - def __init__(self, name: namelike, *terms: Term): + def __init__(self, predicate_name: namelike, *terms: Term): """Initialize the predicate.""" - self._name = parse_name(name) + self._name = parse_name(predicate_name) self._terms = tuple(terms) + _check_terms_consistency(self._terms) @property def name(self) -> str: diff --git a/pddl/logic/terms.py b/pddl/logic/terms.py index 22e578f4..4fb37f15 100644 --- a/pddl/logic/terms.py +++ b/pddl/logic/terms.py @@ -20,26 +20,33 @@ from pddl.helpers.cache_hash import cache_hash +def _print_tag_set(type_tags: AbstractSet[name_type]) -> str: + """Print a tag set.""" + if len(type_tags) == 0: + return "[]" + return repr(sorted(map(str, type_tags))) + + @cache_hash @functools.total_ordering class Term: """A term in a formula.""" def __init__( - self, name: namelike, type_tags: Optional[Collection[namelike]] = None + self, term_name: namelike, type_tags: Optional[Collection[namelike]] = None ): """ Initialize a term. - :param name: the name for the term. + :param term_name: the name for the term. :param type_tags: the type tags associated to this term. """ assert_(type(self) is not Term, "Term is an abstract class") - self._name = parse_name(name) + self._name = parse_name(term_name) self._type_tags = frozenset(to_type(ensure_set(check_no_duplicates(type_tags)))) # type: ignore @property - def name(self) -> str: + def name(self) -> name_type: """Get the name.""" return self._name diff --git a/pddl/parser/typed_list_parser.py b/pddl/parser/typed_list_parser.py index 1bfc6575..2f259eca 100644 --- a/pddl/parser/typed_list_parser.py +++ b/pddl/parser/typed_list_parser.py @@ -16,6 +16,7 @@ from pddl.custom_types import name, parse_name, parse_type from pddl.helpers.base import check, safe_index +from pddl.logic.terms import _print_tag_set from pddl.parser.symbols import Symbols @@ -203,7 +204,7 @@ def _check_item_types(self, item_name: name, type_tags: Set[name]) -> None: if previous_type_tags != type_tags: raise ValueError( f"invalid types for item '{item_name}': previous known tags were " - f"{self._print_tag_set(previous_type_tags)}, got {self._print_tag_set(type_tags)}" + f"{_print_tag_set(previous_type_tags)}, got {_print_tag_set(type_tags)}" ) def _add_item(self, item_name: name, type_tags: Set[name]) -> None: @@ -219,11 +220,5 @@ def _raise_multiple_types_error( """Raise an error if the item has multiple types.""" raise ValueError( f"typed list names should not have more than one type, got '{item_name}' with " - f"types {self._print_tag_set(type_tags)}" + f"types {_print_tag_set(type_tags)}" ) - - def _print_tag_set(self, type_tags: Set[name]) -> str: - """Print a tag set.""" - if len(type_tags) == 0: - return "[]" - return repr(sorted(map(str, type_tags))) diff --git a/tests/test_logic/test_predicates.py b/tests/test_logic/test_predicates.py new file mode 100644 index 00000000..4bdb6f69 --- /dev/null +++ b/tests/test_logic/test_predicates.py @@ -0,0 +1,28 @@ +# +# Copyright 2021-2023 WhiteMech +# +# ------------------------------ +# +# This file is part of pddl. +# +# Use of this source code is governed by an MIT-style +# license that can be found in the LICENSE file or at +# https://opensource.org/licenses/MIT. +# + +"""Test pddl.logic.predicates module.""" +import pytest + +from pddl.logic import Predicate +from pddl.logic.terms import Variable + + +def test_inconsistent_predicate_terms() -> None: + """Test that terms of a predicate must have consistent typing.""" + with pytest.raises( + ValueError, + match=r"Term \?a has inconsistent type tags: previous type tags \['t1', 't2'\], new type tags " + r"\['t3', 't4'\]", + ): + a1, a2 = Variable("a", ["t1", "t2"]), Variable("a", ["t3", "t4"]) + Predicate("p", a1, a2) diff --git a/tests/test_logic/test_terms.py b/tests/test_logic/test_terms.py index 70163327..02082402 100644 --- a/tests/test_logic/test_terms.py +++ b/tests/test_logic/test_terms.py @@ -10,7 +10,7 @@ # https://opensource.org/licenses/MIT. # -"""Test pddl.logic module.""" +"""Test pddl.logic.terms module.""" import pytest from pddl.logic.terms import Variable From 924c88cbc98614952b37d738459ea7dc4a128be0 Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Fri, 9 Jun 2023 16:40:13 +0200 Subject: [PATCH 47/51] feat: add term type checks in EqualTo class --- pddl/logic/predicates.py | 1 + tests/test_logic/test_predicates.py | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/pddl/logic/predicates.py b/pddl/logic/predicates.py index 615bfc8d..dc124bec 100644 --- a/pddl/logic/predicates.py +++ b/pddl/logic/predicates.py @@ -121,6 +121,7 @@ def __init__(self, left: Term, right: Term): """ self._left = left self._right = right + _check_terms_consistency([self._left, self._right]) @property def left(self) -> Term: diff --git a/tests/test_logic/test_predicates.py b/tests/test_logic/test_predicates.py index 4bdb6f69..f032718a 100644 --- a/tests/test_logic/test_predicates.py +++ b/tests/test_logic/test_predicates.py @@ -14,6 +14,7 @@ import pytest from pddl.logic import Predicate +from pddl.logic.predicates import EqualTo from pddl.logic.terms import Variable @@ -26,3 +27,14 @@ def test_inconsistent_predicate_terms() -> None: ): a1, a2 = Variable("a", ["t1", "t2"]), Variable("a", ["t3", "t4"]) Predicate("p", a1, a2) + + +def test_inconsistent_equal_to_terms() -> None: + """Test that terms of a EqualTo atomic must have consistent typing.""" + with pytest.raises( + ValueError, + match=r"Term \?a has inconsistent type tags: previous type tags \['t1', 't2'\], new type tags " + r"\['t3', 't4'\]", + ): + a1, a2 = Variable("a", ["t1", "t2"]), Variable("a", ["t3", "t4"]) + EqualTo(a1, a2) From f7996d12e6f4a570b13eb5264a18ced2bbeacd51 Mon Sep 17 00:00:00 2001 From: MarcoFavorito Date: Fri, 9 Jun 2023 17:13:07 +0200 Subject: [PATCH 48/51] test: add more tests for problem parsing --- tests/test_parser/test_problem.py | 79 +++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/tests/test_parser/test_problem.py b/tests/test_parser/test_problem.py index ba57a13c..3545b6ab 100644 --- a/tests/test_parser/test_problem.py +++ b/tests/test_parser/test_problem.py @@ -13,6 +13,9 @@ """This module contains the tests for the domain parser.""" from textwrap import dedent +import lark +import pytest + from pddl.parser.problem import ProblemParser from pddl.requirements import Requirements @@ -32,3 +35,79 @@ def test_problem_requirements_section_parsed() -> None: problem = ProblemParser()(problem_str) assert problem.requirements == {Requirements.TYPING} + + +def test_problem_objects_repetition_in_simple_typed_lists_not_allowed() -> None: + """Check objects repetition in simple typed lists is detected and a parsing error is raised.""" + problem_str = dedent( + """ + (define (problem test-problem) + (:domain test-domain) + (:requirements :typing) + (:objects a b c a) + (:init ) + (:goal (and )) + ) + """ + ) + + with pytest.raises( + lark.exceptions.VisitError, + match=".*error while parsing tokens \\['a', 'b', 'c', 'a'\\]: " + "duplicate name 'a' in typed list already present", + ): + ProblemParser()(problem_str) + + +def test_problem_objects_repetition_in_typed_lists_not_allowed() -> None: + """Check objects repetition in typed lists is detected and a parsing error is raised.""" + problem_str = dedent( + """ + (define (problem test-problem) + (:domain test-domain) + (:requirements :typing) + (:objects a - t1 b - t2 c - t3 a - t4) + (:init ) + (:goal (and )) + ) + """ + ) + + with pytest.raises( + lark.exceptions.VisitError, + match=r".*error while parsing tokens \['a', '-', 't1', 'b', '-', 't2', 'c', '-', 't3', 'a', '-', 't4'\]: " + r"duplicate name 'a' in typed list already present with types \['t1'\]", + ): + ProblemParser()(problem_str) + + +def test_problem_init_predicate_arg_repetition_allowed() -> None: + """Check argument repetition in predicate list is allowed.""" + problem_str = dedent( + """ + (define (problem test-problem) + (:domain test-domain) + (:requirements :typing) + (:objects a) + (:init (p a a)) + (:goal (and )) + ) + """ + ) + ProblemParser()(problem_str) + + +def test_problem_init_predicate_repetition_name_allowed() -> None: + """Check predicate repetition in init condition is allowed.""" + problem_str = dedent( + """ + (define (problem test-problem) + (:domain test-domain) + (:requirements :typing) + (:objects a) + (:init (p a a) (p a a)) + (:goal (and )) + ) + """ + ) + ProblemParser()(problem_str) From f5858b3faa7f6f468968afea22b9e364e774254f Mon Sep 17 00:00:00 2001 From: Marco Favorito Date: Fri, 9 Jun 2023 22:16:51 +0200 Subject: [PATCH 49/51] ci: change GH action version from master to main --- .github/workflows/docs.yml | 4 ++-- .github/workflows/linting.yml | 4 ++-- .github/workflows/test.yml | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 2c3a8b50..6df720a3 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -18,8 +18,8 @@ jobs: timeout-minutes: 30 steps: - - uses: actions/checkout@master - - uses: actions/setup-python@master + - uses: actions/checkout@main + - uses: actions/setup-python@main with: python-version: ${{ matrix.python-version }} - name: Install dependencies diff --git a/.github/workflows/linting.yml b/.github/workflows/linting.yml index 60678874..bc5bde34 100644 --- a/.github/workflows/linting.yml +++ b/.github/workflows/linting.yml @@ -18,8 +18,8 @@ jobs: timeout-minutes: 30 steps: - - uses: actions/checkout@master - - uses: actions/setup-python@master + - uses: actions/checkout@main + - uses: actions/setup-python@main with: python-version: ${{ matrix.python-version }} - name: Install dependencies diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c9da0db3..bb6fd10f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -18,8 +18,8 @@ jobs: timeout-minutes: 30 steps: - - uses: actions/checkout@master - - uses: actions/setup-python@master + - uses: actions/checkout@main + - uses: actions/setup-python@main with: python-version: ${{ matrix.python-version }} - name: Install dependencies From edcc29547276e239fd945c7c827ac060b6636738 Mon Sep 17 00:00:00 2001 From: Marco Favorito Date: Tue, 13 Jun 2023 10:39:23 +0200 Subject: [PATCH 50/51] fix formatting of typed lists - types are now printed correctly in the :constants section - in the :objects section, the objects are grouped by type (as in constants) - minor fixes in how :init and :goal are printed (they are always printed even if empty). The code changes added here are to be considered temporary, since a refactoring of the formatting module is required. Nevertheless, the printing of the typing information of constants will remain. --- README.md | 4 +- pddl/formatter.py | 99 ++++++++++++++++++++++++++++++----------- tests/test_formatter.py | 63 ++++++++++++++++++++++++++ 3 files changed, 138 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index 0b4c9e5d..caa3e752 100644 --- a/README.md +++ b/README.md @@ -120,7 +120,7 @@ that gives: (define (domain my_domain) (:requirements :strips :typing) (:types type_1) - (:constants a b c) + (:constants a b c - type_1) (:predicates (p1 ?x - type_1 ?y - type_1 ?z - type_1) (p2 ?x - type_1 ?y - type_1)) (:action action-1 :parameters (?x - type_1 ?y - type_1 ?z - type_1) @@ -148,7 +148,7 @@ Output: (define (problem problem-1) (:domain my_domain) (:requirements :strips :typing) - (:objects a - type_1 b - type_1 c - type_1) + (:objects a b c - type_1) (:init (not (p2 b c)) (p1 a b c)) (:goal (p2 b c)) ) diff --git a/pddl/formatter.py b/pddl/formatter.py index c88027ab..8b5a8454 100644 --- a/pddl/formatter.py +++ b/pddl/formatter.py @@ -12,11 +12,12 @@ """Formatting utilities for PDDL domains and problems.""" from textwrap import indent -from typing import Callable, Collection, Dict, Optional +from typing import Callable, Collection, Dict, List, Optional from pddl.core import Domain, Problem from pddl.custom_types import name from pddl.logic.base import TRUE +from pddl.logic.terms import Constant def _remove_empty_lines(s: str) -> str: @@ -25,12 +26,41 @@ def _remove_empty_lines(s: str) -> str: def _sort_and_print_collection( - prefix, collection: Collection, postfix, to_string: Callable = str + prefix, + collection: Collection, + postfix, + to_string: Callable = str, + is_mandatory: bool = False, ): if len(collection) > 0: return prefix + " ".join(sorted(map(to_string, collection))) + postfix - else: - return "" + elif is_mandatory: + return prefix + postfix + return "" + + +def _print_types_with_parents( + prefix: str, + types_dict: Dict[name, Optional[name]], + postfix: str, + to_string: Callable = str, +): + """Print the type dictionary of a PDDL domain.""" + name_by_type: Dict[Optional[name], List[name]] = {} + for type_name, parent_type in types_dict.items(): + name_by_type.setdefault(parent_type, []).append(type_name) + return _print_typed_lists(prefix, name_by_type, postfix, to_string) + + +def _print_constants( + prefix, constants: Collection[Constant], postfix, to_string: Callable = str +): + """Print constants in a PDDL domain.""" + term_by_type_tags: Dict[Optional[name], List[name]] = {} + for c in constants: + term_by_type_tags.setdefault(c.type_tag, []).append(c.name) + + return _print_typed_lists(prefix, term_by_type_tags, postfix, to_string) def _print_predicates_with_types(predicates: Collection): @@ -54,26 +84,37 @@ def _print_predicates_with_types(predicates: Collection): return result.strip() -def _print_types_with_parents(types: Dict[name, Optional[name]]): - """ - Print types with parent types.. +def _print_typed_lists( + prefix, + names_by_type: Dict[Optional[name], List[name]], + postfix, + to_string: Callable = str, +): + """Print typed lists.""" + result = prefix + " " - :param types: the type definition in dict format. - :return: the domain types definition in string format. - """ - result = "" - for t in sorted(types.keys()): - result += f"{t} - {types[t]}" if types[t] else f"{t}" - result += " " - return result.strip() + # names with no type will be printed at the end + names_with_none_types = names_by_type.pop(None, []) + # print typed constants, first sorted by type, then by constant name + for type_tag, typed_names in sorted( + names_by_type.items(), key=lambda type_and_name: type_and_name[0] # type: ignore + ): + result += ( + " ".join(sorted(to_string(n) for n in typed_names)) + " - " + type_tag + " " # type: ignore + ) -def _print_objects_with_types(objects: Collection): - result = "" - for o in sorted(objects): - result += f"{o.name} - {' '.join(o.type_tags)}" if o.type_tags else f"{o.name}" - result += " " - return result.strip() + if len(names_with_none_types) == 0: + return result.strip() + postfix + + # print constants with no type + result += " ".join(sorted(to_string(n) for n in names_with_none_types)) + + if result == prefix + " ": + result = result[:-1] + result += postfix + + return result def domain_to_string(domain: Domain) -> str: @@ -82,8 +123,8 @@ def domain_to_string(domain: Domain) -> str: body = "" indentation = " " * 4 body += _sort_and_print_collection("(:requirements ", domain.requirements, ")\n") - body += f"(:types {_print_types_with_parents(domain.types)})\n" - body += _sort_and_print_collection("(:constants ", domain.constants, ")\n") + body += _print_types_with_parents("(:types", domain.types, ")\n") + body += _print_constants("(:constants", domain.constants, ")\n") body += f"(:predicates {_print_predicates_with_types(domain.predicates)})\n" body += _sort_and_print_collection( "", @@ -108,9 +149,15 @@ def problem_to_string(problem: Problem) -> str: body = f"(:domain {problem.domain_name})\n" indentation = " " * 4 body += _sort_and_print_collection("(:requirements ", problem.requirements, ")\n") - body += f"(:objects {_print_objects_with_types(problem.objects)})\n" - body += _sort_and_print_collection("(:init ", problem.init, ")\n") - body += f"{'(:goal ' + str(problem.goal) + ')'}\n" if problem.goal != TRUE else "" + body += _print_constants("(:objects", problem.objects, ")\n") + body += _sort_and_print_collection( + "(:init ", problem.init, ")\n", is_mandatory=True + ) + body += ( + f"{'(:goal ' + str(problem.goal) + ')'}\n" + if problem.goal != TRUE + else "(:goal (and))\n" + ) result = result + "\n" + indent(body, indentation) + "\n)" result = _remove_empty_lines(result) return result diff --git a/tests/test_formatter.py b/tests/test_formatter.py index 851ca6ea..63bc66ec 100644 --- a/tests/test_formatter.py +++ b/tests/test_formatter.py @@ -13,10 +13,14 @@ """This module contains tests verifying that the formatter outputs are syntactically valid and semantically faithful.""" from pathlib import Path +from textwrap import dedent import pytest +from pddl.core import Domain, Problem from pddl.formatter import domain_to_string, problem_to_string +from pddl.logic import constants +from pddl.requirements import Requirements from tests.conftest import DOMAIN_FILES, PROBLEM_FILES @@ -36,3 +40,62 @@ def test_problem_formatter(problem_parser, pddl_file): actual_problem_str = problem_to_string(expected_problem_obj) actual_problem_obj = problem_parser(actual_problem_str) assert actual_problem_obj == expected_problem_obj + + +def test_typed_constants_formatting_in_domain() -> None: + """Test that types and typed constants are formatted correctly.""" + t1, t2, t3 = "type_1", "type_2", "type_3" + + a1, b1, c1 = constants("a b c", type_=t1) + d2, e2, f2 = constants("d e f", type_=t2) + g3, h3, i3 = constants("g h i", type_=t3) + j, k, lc = constants("j k l", type_=None) + + # define the domain object. + domain = Domain( + "my_domain", + requirements=[Requirements.TYPING], + types={t1: None, t2: t1, t3: t1}, + constants=[a1, b1, c1, d2, e2, f2, g3, h3, i3, j, k, lc], + ) + + domain_str = domain_to_string(domain) + + assert domain_str == dedent( + """\ + (define (domain my_domain) + (:requirements :typing) + (:types type_2 type_3 - type_1 type_1) + (:constants a b c - type_1 d e f - type_2 g h i - type_3 j k l) + (:predicates ) + )""" + ) + + +def test_typed_objects_formatting_in_problem() -> None: + """Test that typed objects are formatted correctly.""" + t1, t2, t3 = "type_1", "type_2", "type_3" + + a1, b1, c1 = constants("a b c", type_=t1) + d2, e2, f2 = constants("d e f", type_=t2) + g3, h3, i3 = constants("g h i", type_=t3) + j, k, lc = constants("j k l", type_=None) + + problem = Problem( + "problem-1", + domain_name="my_domain", + requirements=[Requirements.TYPING], + objects=[a1, b1, c1, d2, e2, f2, g3, h3, i3, j, k, lc], + ) + problem_str = problem_to_string(problem) + + assert problem_str == dedent( + """\ + (define (problem problem-1) + (:domain my_domain) + (:requirements :typing) + (:objects a b c - type_1 d e f - type_2 g h i - type_3 j k l) + (:init ) + (:goal (and)) + )""" + ) From 97b379fc70c61b59cb36cf3dbeb99ea747882dee Mon Sep 17 00:00:00 2001 From: Marco Favorito Date: Tue, 13 Jun 2023 14:14:05 +0200 Subject: [PATCH 51/51] fix: update error message in case a name inherits from multiple types This should make the error more clear to the user: instead of just saying that the name is "already present" in the list, we clarify that the problem is that not only it occurs, but it is because it occurs (again) as inheriting name. --- pddl/parser/typed_list_parser.py | 6 ++++-- tests/test_parser/test_domain.py | 35 +++++++++++++++++++++++++++---- tests/test_parser/test_problem.py | 2 +- 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/pddl/parser/typed_list_parser.py b/pddl/parser/typed_list_parser.py index 2f259eca..9be8302d 100644 --- a/pddl/parser/typed_list_parser.py +++ b/pddl/parser/typed_list_parser.py @@ -175,10 +175,12 @@ def _check_item_name_already_present(self, item_name: name) -> None: """ if item_name in self._item_to_types: types_list = sorted(map(str, self._item_to_types[item_name])) - types_list_str = f" with types {types_list}" if len(types_list) > 0 else "" + if len(types_list) > 0: + raise ValueError( + f"duplicate name '{item_name}' in typed list already inherits from types {types_list}" + ) raise ValueError( f"duplicate name '{item_name}' in typed list already present" - + types_list_str ) def _check_tags_already_present( diff --git a/tests/test_parser/test_domain.py b/tests/test_parser/test_domain.py index d4e2cb19..5019456b 100644 --- a/tests/test_parser/test_domain.py +++ b/tests/test_parser/test_domain.py @@ -56,6 +56,33 @@ def test_hierarchical_types() -> None: } +def test_hierarchical_types_2() -> None: + """Test correct parsing of hierarchical types, Storage domain.""" + domain_str = dedent( + """ + (define (domain logistics) + (:requirements :strips :typing) + (:types hoist surface place area - object + container depot - place + storearea transitarea - area + crate - surface) + ) + """ + ) + domain = DomainParser()(domain_str) + assert domain.types == { + "hoist": "object", + "surface": "object", + "place": "object", + "area": "object", + "container": "place", + "depot": "place", + "storearea": "area", + "transitarea": "area", + "crate": "surface", + } + + def test_types_repetition_in_simple_typed_lists_not_allowed() -> None: """Check types repetition in simple typed lists is detected and a parsing error is raised.""" domain_str = dedent( @@ -69,7 +96,7 @@ def test_types_repetition_in_simple_typed_lists_not_allowed() -> None: with pytest.raises( lark.exceptions.VisitError, - match=".*error while parsing tokens \\['a', 'b', 'c', 'a'\\]: " + match=r".*error while parsing tokens \['a', 'b', 'c', 'a'\]: " "duplicate name 'a' in typed list already present", ): DomainParser()(domain_str) @@ -88,8 +115,8 @@ def test_types_repetition_in_typed_lists_not_allowed() -> None: with pytest.raises( lark.exceptions.VisitError, - match=".*error while parsing tokens \\['a', '-', 't1', 'b', 'c', '-', 't2', 'a', '-', 't3'\\]: " - "duplicate name 'a' in typed list already present with types \\['t1'\\]", + match=r".*error while parsing tokens \['a', '-', 't1', 'b', 'c', '-', 't2', 'a', '-', 't3'\]: " + r"duplicate name 'a' in typed list already inherits from types \['t1'\]", ): DomainParser()(domain_str) @@ -167,7 +194,7 @@ def test_constants_repetition_in_typed_lists_not_allowed() -> None: with pytest.raises( lark.exceptions.VisitError, match=".*error while parsing tokens \\['c1', '-', 't1', 'c1', '-', 't2'\\]: " - "duplicate name 'c1' in typed list already present with types \\['t1'\\]", + "duplicate name 'c1' in typed list already inherits from types \\['t1'\\]", ): DomainParser()(domain_str) diff --git a/tests/test_parser/test_problem.py b/tests/test_parser/test_problem.py index 3545b6ab..e08c1c54 100644 --- a/tests/test_parser/test_problem.py +++ b/tests/test_parser/test_problem.py @@ -76,7 +76,7 @@ def test_problem_objects_repetition_in_typed_lists_not_allowed() -> None: with pytest.raises( lark.exceptions.VisitError, match=r".*error while parsing tokens \['a', '-', 't1', 'b', '-', 't2', 'c', '-', 't3', 'a', '-', 't4'\]: " - r"duplicate name 'a' in typed list already present with types \['t1'\]", + r"duplicate name 'a' in typed list already inherits from types \['t1'\]", ): ProblemParser()(problem_str)