Parse Performables and create an Ontology from them #285
Conversation
| @@ -0,0 +1,207 @@ | |||
| from enum import EnumMeta | |||
| from pycram.designators.action_designator import ActionAbstract | |||
There was a problem hiding this comment.
Please make this a relative import: "from ..designators.action_designator ..."
| from random_events.utils import recursive_subclasses | ||
| from owlready2 import * | ||
| from random_events.utils import get_full_class_name | ||
| from typing import Dict, List, Type, Optional, Any, get_origin, Union, get_args |
There was a problem hiding this comment.
Please use typing_extensions for forward compatibility
| """ | ||
| Docstring of the parameter itself (individual to each performable). | ||
| """ | ||
| parameter_default_value: Any |
There was a problem hiding this comment.
Typing should be Optional[Any]
| :return: A list containing the string representation of the default value or | ||
| `None` if no default value exists. | ||
| """ | ||
| if self.parameter_default_value == inspect.Parameter.empty: |
There was a problem hiding this comment.
You could also write this as a one-liner:
return None if self.parameter_default_value == inspect.Parameter.empty else [str(self.parameter_default_value)]
|
|
||
| def create_ontology_from_performables(outputfile = "performables.owl"): | ||
| """ | ||
| Creates an ontology from the performables. |
There was a problem hiding this comment.
Doc string for the parameter and typing for the parameter
| """ | ||
| Creates an ontology from the performables. | ||
| """ | ||
| def unwrap_classname(parameter: ParameterDigest) -> str: |
There was a problem hiding this comment.
Is there a specific reason for the nested methods? Because the get_optional_type seems similar to is_optional_type above
Also, newlines in the docstrings
| :param text: Text with spaces. | ||
| :return: Text without spaces. | ||
| """ | ||
| return text.replace(" ", "") |
There was a problem hiding this comment.
I'm not a huge fan of these one-line methods since they mostly bloat the code and usually the one-line is understandable as is
There was a problem hiding this comment.
yeah me neither. And have you ever heard of "ABC DEF".strip()?
There was a problem hiding this comment.
strip() does only remove leading/trailing whitespace.
| from enum import EnumMeta | ||
| from pycram.designators.action_designator import ActionAbstract | ||
| from random_events.utils import recursive_subclasses | ||
| from owlready2 import * |
There was a problem hiding this comment.
Just a headsup, owlready2 also has a class called World. But if you ever use the pycram_world in this class this will lead to conflicts.
So maybe explicitly import the stuff you need.
| class has_description(DataProperty): | ||
| range = [str] | ||
|
|
||
| # Creation of ontological classes for the parameters and classes for enums |
There was a problem hiding this comment.
I think some of these code can be its own method, the whole method seems a bit long
| """ | ||
| Name of the parameter. | ||
| """ | ||
| docstring_of_parameter_clazz: str |
There was a problem hiding this comment.
format the file usign ctrl+alt+shift+L
|
|
||
| def get_default_value(self) -> Optional[List[str]]: | ||
| """ | ||
| Returns the default value of the parameter if it exists. Else None. |
There was a problem hiding this comment.
the docstring is doppelt gemoppelt. If it can bhe described by the row ":return:" do it only there.
|
|
||
| class ActionAbstractDigest: | ||
| """ | ||
| Wraps all information about an action abstract class that are necessary for the created ontology. |
There was a problem hiding this comment.
docstring should be in imperative!
|
|
||
| def __init__(self, clazz: Type[ActionAbstract]): | ||
| self.clazz: Type[ActionAbstract] = clazz | ||
| self.full_name: str = get_full_class_name(clazz) |
There was a problem hiding this comment.
why is this not a dataclass? looks very dataclass to me.
There was a problem hiding this comment.
Imo it has too much logic encapsulated to be considered a dataclass.
| :param text: Text with spaces. | ||
| :return: Text without spaces. | ||
| """ | ||
| return text.replace(" ", "") |
There was a problem hiding this comment.
yeah me neither. And have you ever heard of "ABC DEF".strip()?
| class has_description(DataProperty): | ||
| range = [str] | ||
|
|
||
| # Creation of ontological classes for the parameters and classes for enums |
| tmpfile = NamedTemporaryFile(mode="w+", suffix=".owl") | ||
| create_ontology_from_performables(outputfile=tmpfile.name) | ||
| ontology = owlready2.get_ontology(tmpfile.name).load() | ||
| performable_class = next(filter(lambda c: c._name == 'Performable', ontology.classes())) |
There was a problem hiding this comment.
use onto.search to get those.
| create_ontology_from_performables(outputfile=tmpfile.name) | ||
| ontology = owlready2.get_ontology(tmpfile.name).load() | ||
| performable_class = next(filter(lambda c: c._name == 'Performable', ontology.classes())) | ||
| self.assertTrue(len(performable_class.instances()) > 0) |
There was a problem hiding this comment.
Be more precise here. This will pass if the parsing is wrong. Assert some more structure and only test for one designator. This will make the test more relevant and show early what breaks.
…subclasses of it.
…a correct for correct extraction.
| """ | ||
| Encapsulation of meta information about a parameter. | ||
| """ | ||
| clazz: Any |
There was a problem hiding this comment.
isnt the type here Type?
There was a problem hiding this comment.
I aligned the type hint with the type hint of Action.get_type_hints(cls) -> Dict[str,Any]. Since I am using the value of this output Dict as-is (without any restricting guards), I chose to stick with "Any" aswell.
| """ | ||
| Class of the parameter. | ||
| """ | ||
| parameter_name: str |
There was a problem hiding this comment.
parameterdigest.parameter_name is overexpressive. Call it just name.
| """ | ||
| Name of the parameter. | ||
| """ | ||
| docstring_of_parameter_clazz: str |
There was a problem hiding this comment.
parameter is redundandent. Call it clazz_docstring or something.
| Holds the default value of the parameter if set. | ||
| """ | ||
| is_enum: bool | ||
| """ |
There was a problem hiding this comment.
sounds like a calculated thing. use a property in the case its calculated.
| docstring_of_parameter_clazz=param_clazz.__doc__, | ||
| docstring_of_parameter=class_param_comment[param], | ||
| parameter_default_value=parameters_inspection[param].default, | ||
| is_enum=param_clazz.__class__ == EnumMeta, |
There was a problem hiding this comment.
yeah thats the thing that can be moved to a property of parameterdigest.
There was a problem hiding this comment.
filenames should be snake_case
| BOTH = 2 | ||
|
|
||
| @dataclass | ||
| class TestOntomaticPerformable(ActionAbstract): |
There was a problem hiding this comment.
Call it OntomaticPerformableTestDummy or some name that indicates that this is a dummy object. Otherwise this sounds like a test case from its name only.
Adds a Module that parses the ActionAbstract subclasses and creates an ontology from them.
The created ontology includes the signature of the constructors of the ActionAbstracts, including the docstring and the type hints. For each parameter of the signature it also parses the docstring into the created ontology.