From badbe759c295309e1ae8fe2a410c847a7fc58243 Mon Sep 17 00:00:00 2001 From: Michael Carlstrom Date: Sun, 1 Sep 2024 22:09:26 -0400 Subject: [PATCH 1/4] add types Signed-off-by: Michael Carlstrom --- .../rosidl_generator_c/__init__.py | 6 ++- .../rosidl_generator_cpp/__init__.py | 5 +- rosidl_pycommon/package.xml | 1 + rosidl_pycommon/py.typed | 0 rosidl_pycommon/rosidl_pycommon/__init__.py | 47 ++++++++++++------- rosidl_pycommon/test/test_copyright.py | 2 +- rosidl_pycommon/test/test_flake8.py | 2 +- rosidl_pycommon/test/test_mypy.py | 23 +++++++++ rosidl_pycommon/test/test_pep257.py | 2 +- .../__init__.py | 4 +- .../__init__.py | 4 +- .../cli.py | 3 +- 12 files changed, 72 insertions(+), 27 deletions(-) create mode 100644 rosidl_pycommon/py.typed create mode 100644 rosidl_pycommon/test/test_mypy.py diff --git a/rosidl_generator_c/rosidl_generator_c/__init__.py b/rosidl_generator_c/rosidl_generator_c/__init__.py index 50bb60b9e..4e4bf76d9 100644 --- a/rosidl_generator_c/rosidl_generator_c/__init__.py +++ b/rosidl_generator_c/rosidl_generator_c/__init__.py @@ -12,6 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +from typing import List + from rosidl_generator_type_description import parse_rihs_string from rosidl_generator_type_description import RIHS01_HASH_VALUE_SIZE from rosidl_parser.definition import AbstractGenericString @@ -28,7 +30,7 @@ from rosidl_pycommon import generate_files -def generate_c(generator_arguments_file, disable_description_codegen=False): +def generate_c(generator_arguments_file, disable_description_codegen: bool = False) -> List[str]: mapping = { 'idl.h.em': '%s.h', 'idl__description.c.em': 'detail/%s__description.c', @@ -46,7 +48,7 @@ def generate_c(generator_arguments_file, disable_description_codegen=False): }) -def prefix_with_bom_if_necessary(content): +def prefix_with_bom_if_necessary(content: str) -> str: try: content.encode('ASCII') except UnicodeError: diff --git a/rosidl_generator_cpp/rosidl_generator_cpp/__init__.py b/rosidl_generator_cpp/rosidl_generator_cpp/__init__.py index b076fd3ed..85db597c7 100644 --- a/rosidl_generator_cpp/rosidl_generator_cpp/__init__.py +++ b/rosidl_generator_cpp/rosidl_generator_cpp/__init__.py @@ -13,6 +13,7 @@ # limitations under the License. from ast import literal_eval +from typing import List from rosidl_parser.definition import AbstractGenericString from rosidl_parser.definition import AbstractNestedType @@ -28,7 +29,7 @@ from rosidl_pycommon import generate_files -def generate_cpp(generator_arguments_file): +def generate_cpp(generator_arguments_file) -> List[str]: mapping = { 'idl.hpp.em': '%s.hpp', 'idl__builder.hpp.em': 'detail/%s__builder.hpp', @@ -41,7 +42,7 @@ def generate_cpp(generator_arguments_file): post_process_callback=prefix_with_bom_if_necessary) -def prefix_with_bom_if_necessary(content): +def prefix_with_bom_if_necessary(content: str) -> str: try: content.encode('ASCII') except UnicodeError: diff --git a/rosidl_pycommon/package.xml b/rosidl_pycommon/package.xml index 9467b4a68..30a49c26b 100644 --- a/rosidl_pycommon/package.xml +++ b/rosidl_pycommon/package.xml @@ -20,6 +20,7 @@ ament_copyright ament_flake8 ament_pep257 + ament_mypy python3-pytest diff --git a/rosidl_pycommon/py.typed b/rosidl_pycommon/py.typed new file mode 100644 index 000000000..e69de29bb diff --git a/rosidl_pycommon/rosidl_pycommon/__init__.py b/rosidl_pycommon/rosidl_pycommon/__init__.py index 36d887113..16e598462 100644 --- a/rosidl_pycommon/rosidl_pycommon/__init__.py +++ b/rosidl_pycommon/rosidl_pycommon/__init__.py @@ -18,6 +18,7 @@ import pathlib import re import sys +from typing import Any, Callable, Dict, List, Optional, TYPE_CHECKING import em @@ -31,7 +32,11 @@ from rosidl_parser.parser import parse_idl_file -def convert_camel_case_to_lower_case_underscore(value): +if TYPE_CHECKING: + from _typeshed import FileDescriptorOrPath + + +def convert_camel_case_to_lower_case_underscore(value: str) -> str: # insert an underscore before any upper case letter # which is followed by a lower case letter value = re.sub('(.)([A-Z][a-z]+)', r'\1_\2', value) @@ -41,12 +46,14 @@ def convert_camel_case_to_lower_case_underscore(value): return value.lower() -def read_generator_arguments(input_file): +def read_generator_arguments(input_file: 'FileDescriptorOrPath') -> Any: with open(input_file, mode='r', encoding='utf-8') as h: return json.load(h) -def get_newest_modification_time(target_dependencies): +def get_newest_modification_time( + target_dependencies: List['FileDescriptorOrPath'] +) -> Optional[float]: newest_timestamp = None for dep in target_dependencies: ts = os.path.getmtime(dep) @@ -56,9 +63,10 @@ def get_newest_modification_time(target_dependencies): def generate_files( - generator_arguments_file, mapping, additional_context=None, - keep_case=False, post_process_callback=None -): + generator_arguments_file: 'FileDescriptorOrPath', mapping: Dict[str, str], + additional_context: Optional[Dict[str, bool]] = None, + keep_case: bool = False, post_process_callback: Optional[Callable[[str], str]] = None +) -> List[str]: args = read_generator_arguments(generator_arguments_file) template_basepath = pathlib.Path(args['template_dir']) @@ -67,7 +75,7 @@ def generate_files( 'Could not find template: ' + template_filename latest_target_timestamp = get_newest_modification_time(args['target_dependencies']) - generated_files = [] + generated_files: List[str] = [] type_description_files = {} for description_tuple in args.get('type_description_tuples', []): @@ -128,10 +136,10 @@ def generate_files( return generated_files -template_prefix_path = [] +template_prefix_path: List[pathlib.Path] = [] -def get_template_path(template_name): +def get_template_path(template_name: str) -> pathlib.Path: global template_prefix_path for basepath in template_prefix_path: template_path = basepath / template_name @@ -144,14 +152,16 @@ def get_template_path(template_name): def expand_template( - template_name, data, output_file, minimum_timestamp=None, - template_basepath=None, post_process_callback=None -): + template_name: str, data: Dict[str, Any], output_file: str, + minimum_timestamp: Optional[float] = None, + template_basepath: Optional[pathlib.Path] = None, + post_process_callback: Optional[Callable[[str], str]] = None +) -> None: # in the legacy API the first argument was the path to the template if template_basepath is None: - template_name = pathlib.Path(template_name) - template_basepath = template_name.parent - template_name = template_name.name + template_path = pathlib.Path(template_name) + template_basepath = template_path.parent + template_name = template_path.name global template_prefix_path template_prefix_path.append(template_basepath) @@ -226,14 +236,17 @@ def expand_template( h.write(content) -def _add_helper_functions(data): +def _add_helper_functions(data: Dict[str, Any]) -> None: data['TEMPLATE'] = _expand_template -def _expand_template(template_name, **kwargs): +def _expand_template(template_name: str, **kwargs: Any) -> None: global interpreter template_path = get_template_path(template_name) _add_helper_functions(kwargs) + if interpreter is None: + raise RuntimeError('_expand_template called before expand_template') + with template_path.open('r') as h: interpreter.invoke( 'beforeInclude', name=str(template_path), file=h, locals=kwargs) diff --git a/rosidl_pycommon/test/test_copyright.py b/rosidl_pycommon/test/test_copyright.py index a89fa2726..2db7dabb1 100644 --- a/rosidl_pycommon/test/test_copyright.py +++ b/rosidl_pycommon/test/test_copyright.py @@ -18,6 +18,6 @@ @pytest.mark.copyright @pytest.mark.linter -def test_copyright(): +def test_copyright() -> None: rc = main(argv=['.', 'test']) assert rc == 0, 'Found errors' diff --git a/rosidl_pycommon/test/test_flake8.py b/rosidl_pycommon/test/test_flake8.py index 648bb70f9..7830616b3 100644 --- a/rosidl_pycommon/test/test_flake8.py +++ b/rosidl_pycommon/test/test_flake8.py @@ -18,7 +18,7 @@ @pytest.mark.flake8 @pytest.mark.linter -def test_flake8(): +def test_flake8() -> None: rc, errors = main_with_errors(argv=[]) assert rc == 0, \ 'Found %d code style errors / warnings:\n' % len(errors) + \ diff --git a/rosidl_pycommon/test/test_mypy.py b/rosidl_pycommon/test/test_mypy.py new file mode 100644 index 000000000..97e4f502a --- /dev/null +++ b/rosidl_pycommon/test/test_mypy.py @@ -0,0 +1,23 @@ +# Copyright 2024 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from ament_mypy.main import main +import pytest + + +@pytest.mark.mypy +@pytest.mark.linter +def test_mypy() -> None: + rc = main(argv=[]) + assert rc == 0, 'Found type errors!' diff --git a/rosidl_pycommon/test/test_pep257.py b/rosidl_pycommon/test/test_pep257.py index 7a2a3747e..d1d2b8e95 100644 --- a/rosidl_pycommon/test/test_pep257.py +++ b/rosidl_pycommon/test/test_pep257.py @@ -18,6 +18,6 @@ @pytest.mark.linter @pytest.mark.pep257 -def test_pep257(): +def test_pep257() -> None: rc = main(argv=['.', 'test']) assert rc == 0, 'Found code style errors / warnings' diff --git a/rosidl_typesupport_introspection_c/rosidl_typesupport_introspection_c/__init__.py b/rosidl_typesupport_introspection_c/rosidl_typesupport_introspection_c/__init__.py index 8a728f9a5..ab96f68d2 100644 --- a/rosidl_typesupport_introspection_c/rosidl_typesupport_introspection_c/__init__.py +++ b/rosidl_typesupport_introspection_c/rosidl_typesupport_introspection_c/__init__.py @@ -12,10 +12,12 @@ # See the License for the specific language governing permissions and # limitations under the License. +from typing import List + from rosidl_pycommon import generate_files -def generate_c(generator_arguments_file: str): +def generate_c(generator_arguments_file: str) -> List[str]: """ Generate the C implementation of the type support. diff --git a/rosidl_typesupport_introspection_cpp/rosidl_typesupport_introspection_cpp/__init__.py b/rosidl_typesupport_introspection_cpp/rosidl_typesupport_introspection_cpp/__init__.py index 7457e2a60..b4ba5ed45 100644 --- a/rosidl_typesupport_introspection_cpp/rosidl_typesupport_introspection_cpp/__init__.py +++ b/rosidl_typesupport_introspection_cpp/rosidl_typesupport_introspection_cpp/__init__.py @@ -12,10 +12,12 @@ # See the License for the specific language governing permissions and # limitations under the License. +from typing import List + from rosidl_pycommon import generate_files -def generate_cpp(generator_arguments_file: str): +def generate_cpp(generator_arguments_file: str) -> List[str]: """ Generate the C++ implementation of the type support. diff --git a/rosidl_typesupport_introspection_cpp/rosidl_typesupport_introspection_cpp/cli.py b/rosidl_typesupport_introspection_cpp/rosidl_typesupport_introspection_cpp/cli.py index a26b5d99d..d9091680d 100644 --- a/rosidl_typesupport_introspection_cpp/rosidl_typesupport_introspection_cpp/cli.py +++ b/rosidl_typesupport_introspection_cpp/rosidl_typesupport_introspection_cpp/cli.py @@ -13,6 +13,7 @@ # limitations under the License. import pathlib +from typing import List from ament_index_python import get_package_share_directory @@ -32,7 +33,7 @@ def generate( interface_files, include_paths, output_path - ): + ) -> List[str]: package_share_path = pathlib.Path( get_package_share_directory('rosidl_typesupport_introspection_cpp')) From 9bbca0048c9ba30ded6d421722346b63718f79aa Mon Sep 17 00:00:00 2001 From: Michael Carlstrom Date: Thu, 5 Sep 2024 23:14:04 -0400 Subject: [PATCH 2/4] use hasattr check Signed-off-by: Michael Carlstrom --- rosidl_pycommon/rosidl_pycommon/__init__.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/rosidl_pycommon/rosidl_pycommon/__init__.py b/rosidl_pycommon/rosidl_pycommon/__init__.py index 16e598462..c123f4829 100644 --- a/rosidl_pycommon/rosidl_pycommon/__init__.py +++ b/rosidl_pycommon/rosidl_pycommon/__init__.py @@ -33,8 +33,14 @@ if TYPE_CHECKING: - from _typeshed import FileDescriptorOrPath + from typing_extensions import TypeAlias + import _typeshed + if hasattr(_typeshed, "FileDescriptorOrPath"): + from _typeshed import FileDescriptorOrPath + else: + # Done since Windows and RHEL uses a mypy too old have FileDescriptorOrPath + FileDescriptorOrPath: TypeAlias = Any #type: ignore[misc, no-redef] def convert_camel_case_to_lower_case_underscore(value: str) -> str: # insert an underscore before any upper case letter From 2dcbfd572d2ccf74cd6d90d1ef7e1b3cf25e4c42 Mon Sep 17 00:00:00 2001 From: Michael Carlstrom Date: Thu, 5 Sep 2024 23:27:22 -0400 Subject: [PATCH 3/4] flake8 Signed-off-by: Michael Carlstrom --- rosidl_pycommon/rosidl_pycommon/__init__.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/rosidl_pycommon/rosidl_pycommon/__init__.py b/rosidl_pycommon/rosidl_pycommon/__init__.py index c123f4829..a86ef6fba 100644 --- a/rosidl_pycommon/rosidl_pycommon/__init__.py +++ b/rosidl_pycommon/rosidl_pycommon/__init__.py @@ -36,11 +36,12 @@ from typing_extensions import TypeAlias import _typeshed - if hasattr(_typeshed, "FileDescriptorOrPath"): - from _typeshed import FileDescriptorOrPath + if hasattr(_typeshed, 'FileDescriptorOrPath'): + from _typeshed import FileDescriptorOrPath else: # Done since Windows and RHEL uses a mypy too old have FileDescriptorOrPath - FileDescriptorOrPath: TypeAlias = Any #type: ignore[misc, no-redef] + FileDescriptorOrPath: TypeAlias = Any # type: ignore[misc, no-redef] + def convert_camel_case_to_lower_case_underscore(value: str) -> str: # insert an underscore before any upper case letter From c428d025e2323194d5a55b683462b0c852d1d224 Mon Sep 17 00:00:00 2001 From: Michael Carlstrom Date: Sat, 7 Sep 2024 11:58:26 -0400 Subject: [PATCH 4/4] use str instead Signed-off-by: Michael Carlstrom --- rosidl_pycommon/rosidl_pycommon/__init__.py | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/rosidl_pycommon/rosidl_pycommon/__init__.py b/rosidl_pycommon/rosidl_pycommon/__init__.py index a86ef6fba..d52ac9608 100644 --- a/rosidl_pycommon/rosidl_pycommon/__init__.py +++ b/rosidl_pycommon/rosidl_pycommon/__init__.py @@ -18,7 +18,7 @@ import pathlib import re import sys -from typing import Any, Callable, Dict, List, Optional, TYPE_CHECKING +from typing import Any, Callable, Dict, List, Optional import em @@ -32,17 +32,6 @@ from rosidl_parser.parser import parse_idl_file -if TYPE_CHECKING: - from typing_extensions import TypeAlias - import _typeshed - - if hasattr(_typeshed, 'FileDescriptorOrPath'): - from _typeshed import FileDescriptorOrPath - else: - # Done since Windows and RHEL uses a mypy too old have FileDescriptorOrPath - FileDescriptorOrPath: TypeAlias = Any # type: ignore[misc, no-redef] - - def convert_camel_case_to_lower_case_underscore(value: str) -> str: # insert an underscore before any upper case letter # which is followed by a lower case letter @@ -53,13 +42,13 @@ def convert_camel_case_to_lower_case_underscore(value: str) -> str: return value.lower() -def read_generator_arguments(input_file: 'FileDescriptorOrPath') -> Any: +def read_generator_arguments(input_file: str) -> Any: with open(input_file, mode='r', encoding='utf-8') as h: return json.load(h) def get_newest_modification_time( - target_dependencies: List['FileDescriptorOrPath'] + target_dependencies: List[str] ) -> Optional[float]: newest_timestamp = None for dep in target_dependencies: @@ -70,7 +59,7 @@ def get_newest_modification_time( def generate_files( - generator_arguments_file: 'FileDescriptorOrPath', mapping: Dict[str, str], + generator_arguments_file: str, mapping: Dict[str, str], additional_context: Optional[Dict[str, bool]] = None, keep_case: bool = False, post_process_callback: Optional[Callable[[str], str]] = None ) -> List[str]: