From f250d4594eefd0a967be565b12536df33efd683a Mon Sep 17 00:00:00 2001 From: James Prestwood Date: Tue, 2 Jul 2024 05:52:57 -0700 Subject: [PATCH] Package wide linting fixes (#2) --- .../cmake/ament_generate_virtualenv.cmake | 18 ++++++++++++++---- .../ament_virtualenv/combine_requirements.py | 9 ++++++--- .../ament_virtualenv/glob_requirements.py | 6 ++++-- ament_virtualenv/ament_virtualenv/package.py | 18 ++++++++++++------ 4 files changed, 36 insertions(+), 15 deletions(-) diff --git a/ament_cmake_virtualenv/cmake/ament_generate_virtualenv.cmake b/ament_cmake_virtualenv/cmake/ament_generate_virtualenv.cmake index 8a4522b..c1820c1 100644 --- a/ament_cmake_virtualenv/cmake/ament_generate_virtualenv.cmake +++ b/ament_cmake_virtualenv/cmake/ament_generate_virtualenv.cmake @@ -139,8 +139,13 @@ function(ament_generate_virtualenv) _ament_cmake_python_install_module(${ARGN}) get_filename_component(module_path ${ARGN} NAME) set(module_path "${CMAKE_INSTALL_PREFIX}/${PYTHON_INSTALL_DIR}/${module_path}") - # message(WARNING "[ament_cmake_virtualenv]: ament_python_install_module override for ${module_path} to ${${PROJECT_NAME}_VENV_INSTALL_DIR}") - install(CODE "execute_process(COMMAND ${wrap_module_BIN} --module-path ${module_path} --venv-install-dir ${${PROJECT_NAME}_VENV_INSTALL_DIR})") + # message( + # WARNING "[ament_cmake_virtualenv]: ament_python_install_module override for ${module_path} to ${${PROJECT_NAME}_VENV_INSTALL_DIR}" + # ) + install( + CODE "execute_process(\ +COMMAND ${wrap_module_BIN} --module-path ${module_path} --venv-install-dir ${${PROJECT_NAME}_VENV_INSTALL_DIR})" + ) endmacro() # Override the ament_python_install_package macro to wrap packages @@ -153,7 +158,12 @@ function(ament_generate_virtualenv) _ament_cmake_python_register_environment_hook() _ament_cmake_python_install_package(${ARGN}) set(package_dir "${CMAKE_INSTALL_PREFIX}/${PYTHON_INSTALL_DIR}/${ARGN}") - # message(WARNING "[ament_cmake_virtualenv]: ament_python_install_package override for ${package_dir} to ${${PROJECT_NAME}_VENV_INSTALL_DIR}") - install(CODE "execute_process(COMMAND ${wrap_package_BIN} --package-dir ${package_dir} --venv-install-dir ${${PROJECT_NAME}_VENV_INSTALL_DIR})") + # message( + # WARNING "[ament_cmake_virtualenv]: ament_python_install_package override for ${package_dir} to ${${PROJECT_NAME}_VENV_INSTALL_DIR}" + # ) + install( + CODE "execute_process(\ +COMMAND ${wrap_package_BIN} --package-dir ${package_dir} --venv-install-dir ${${PROJECT_NAME}_VENV_INSTALL_DIR})" + ) endmacro() endfunction() diff --git a/ament_virtualenv/ament_virtualenv/combine_requirements.py b/ament_virtualenv/ament_virtualenv/combine_requirements.py index 16c8340..57b3e76 100755 --- a/ament_virtualenv/ament_virtualenv/combine_requirements.py +++ b/ament_virtualenv/ament_virtualenv/combine_requirements.py @@ -26,6 +26,8 @@ import re import sys +from typing import List, Dict +from io import TextIOWrapper from collections import namedtuple from packaging.requirements import Requirement, InvalidRequirement @@ -43,9 +45,10 @@ SuppressedRequirement = namedtuple("SuppressedRequirement", "requirement source") -def combine_requirements(requirements_list, output_file): - # type: (List[IO], IO) -> int - combined_requirements = {} # type: Dict[str, requirements.Requirement] +def combine_requirements( + requirements_list: List[TextIOWrapper], output_file: TextIOWrapper +) -> int: + combined_requirements: Dict[str, Requirement] = {} for requirements_file in requirements_list: contents = requirements_file.read() diff --git a/ament_virtualenv/ament_virtualenv/glob_requirements.py b/ament_virtualenv/ament_virtualenv/glob_requirements.py index 9b756be..ef993e3 100755 --- a/ament_virtualenv/ament_virtualenv/glob_requirements.py +++ b/ament_virtualenv/ament_virtualenv/glob_requirements.py @@ -26,6 +26,9 @@ import sys import os +from typing import List +from catkin_pkg.package import Package + try: from ament_virtualenv.package import parse_package except ImportError: @@ -112,8 +115,7 @@ def find_in_workspaces(project, file, workspaces=[]): AMENT_VIRTUALENV_TAGNAME = "pip_requirements" -def parse_exported_requirements(package): - # type: (catkin_pkg.package.Package) -> List[str] +def parse_exported_requirements(package: Package) -> List[str]: requirements_list = [] for export in package.exports: if export.tagname == AMENT_VIRTUALENV_TAGNAME: diff --git a/ament_virtualenv/ament_virtualenv/package.py b/ament_virtualenv/ament_virtualenv/package.py index 6adc67b..e982c8e 100644 --- a/ament_virtualenv/ament_virtualenv/package.py +++ b/ament_virtualenv/ament_virtualenv/package.py @@ -103,7 +103,10 @@ def __init__(self, filename=None, **kwargs): slot.append(deepcopy(d)) del kwargs['run_depends'] self.filename = filename - self.licenses = [l if isinstance(l, License) else License(l) for l in self.licenses] + self.licenses = [ + licence if isinstance(licence, License) else + License(licence) for licence in self.licenses + ] # verify that no unknown keywords are passed unknown = set(kwargs.keys()).difference(self.__slots__) if unknown: @@ -217,7 +220,7 @@ def validate(self, warnings=None): 'with a lower case letter and only contain lower case letters, digits, ' 'underscores, and dashes.' % self.name) - version_regexp = '^[0-9]+\.[0-9]+\.[0-9]+$' + version_regexp = '^[0-9]+\.[0-9]+\.[0-9]+$' # noqa: W605 if not self.version: errors.append('Package version must not be empty') elif not re.match(version_regexp, self.version): @@ -225,7 +228,10 @@ def validate(self, warnings=None): 'Package version "%s" does not follow version conventions' % self.version ) - elif not re.match('^(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)$', self.version): + elif not re.match( + '^(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)$', # noqa: W605 + self.version + ): new_warnings.append( 'Package "%s" does not follow the version conventions. ' 'It should not contain leading zeros (unless the number is 0).' % self.name @@ -251,7 +257,7 @@ def validate(self, warnings=None): if not self.licenses: errors.append('The package node must contain at least one "license" tag') - if [l for l in self.licenses if not l.strip()]: + if [license for license in self.licenses if not license.strip()]: errors.append('The license tag must neither be empty nor only contain whitespaces') if self.authors is not None: @@ -389,8 +395,8 @@ def validate(self): if self.email is None: return _pattern = ( - '^[-a-zA-Z0-9_%+]+(\.[-a-zA-Z0-9_%+]+)*' - '@[-a-zA-Z0-9%]+(\.[-a-zA-Z0-9%]+)*\.[a-zA-Z]{2,}$' + '^[-a-zA-Z0-9_%+]+(\.[-a-zA-Z0-9_%+]+)*' # noqa: W605 + '@[-a-zA-Z0-9%]+(\.[-a-zA-Z0-9%]+)*\.[a-zA-Z]{2,}$' # noqa: W605 ) if not re.match(_pattern, self.email): raise InvalidPackage('Invalid email "%s" for person "%s"' % (self.email, self.name))