From 600f9bfbd1eae257ac6ed11827b05049e9317b64 Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Tue, 3 Dec 2024 17:56:20 +1100 Subject: [PATCH] Addressed documentation issues raised in review. --- source/fab/steps/compile_c.py | 2 ++ source/fab/steps/compile_fortran.py | 4 ++++ source/fab/tools/compiler.py | 4 ++-- source/fab/tools/compiler_wrapper.py | 3 +++ .../unit_tests/tools/test_compiler_wrapper.py | 22 +++++++++---------- 5 files changed, 22 insertions(+), 13 deletions(-) diff --git a/source/fab/steps/compile_c.py b/source/fab/steps/compile_c.py index 283d9607..c7d014d7 100644 --- a/source/fab/steps/compile_c.py +++ b/source/fab/steps/compile_c.py @@ -127,6 +127,8 @@ def _compile_file(arg: Tuple[AnalysedC, MpCommonArgs]): if compiler.category != Category.C_COMPILER: raise RuntimeError(f"Unexpected tool '{compiler.name}' of category " f"'{compiler.category}' instead of CCompiler") + # Tool box returns a Tool, in order to make mypy happy, we need + # to cast it to be a Compiler. compiler = cast(Compiler, compiler) with Timer() as timer: flags = Flags(mp_payload.flags.flags_for_path(path=analysed_file.fpath, diff --git a/source/fab/steps/compile_fortran.py b/source/fab/steps/compile_fortran.py index fe6f479b..e5767fae 100644 --- a/source/fab/steps/compile_fortran.py +++ b/source/fab/steps/compile_fortran.py @@ -136,6 +136,8 @@ def handle_compiler_args(config: BuildConfig, common_flags=None, if compiler.category != Category.FORTRAN_COMPILER: raise RuntimeError(f"Unexpected tool '{compiler.name}' of category " f"'{compiler.category}' instead of FortranCompiler") + # The ToolBox returns a Tool. In order to make mypy happy, we need to + # cast this to become a Compiler. compiler = cast(Compiler, compiler) logger.info( f'Fortran compiler is {compiler} {compiler.get_version_string()}') @@ -268,6 +270,8 @@ def process_file(arg: Tuple[AnalysedFortran, MpCommonArgs]) \ raise RuntimeError(f"Unexpected tool '{compiler.name}' of " f"category '{compiler.category}' instead of " f"FortranCompiler") + # The ToolBox returns a Tool, but we need to tell mypy that + # this is a Compiler compiler = cast(Compiler, compiler) flags = Flags(mp_common_args.flags.flags_for_path( path=analysed_file.fpath, config=config)) diff --git a/source/fab/tools/compiler.py b/source/fab/tools/compiler.py index e1f87271..7a635d78 100644 --- a/source/fab/tools/compiler.py +++ b/source/fab/tools/compiler.py @@ -64,7 +64,7 @@ def __init__(self, name: str, @property def mpi(self) -> bool: - '''Returns whether this compiler supports MPI or not.''' + ''':returns: whether this compiler supports MPI or not.''' return self._mpi @property @@ -75,7 +75,7 @@ def openmp(self) -> bool: @property def openmp_flag(self) -> str: - '''Returns the flag to enable OpenMP.''' + ''':returns: the flag to enable OpenMP.''' return self._openmp_flag def get_hash(self) -> int: diff --git a/source/fab/tools/compiler_wrapper.py b/source/fab/tools/compiler_wrapper.py index ad76bfad..19186b2d 100644 --- a/source/fab/tools/compiler_wrapper.py +++ b/source/fab/tools/compiler_wrapper.py @@ -151,6 +151,9 @@ def compile_file(self, input_file: Path, a syntax check ''' + # TODO #370: replace change_exec_name, and instead provide + # a function that returns the whole command line, which can + # then be modified here. orig_compiler_name = self._compiler.exec_name self._compiler.change_exec_name(self.exec_name) if add_flags is None: diff --git a/tests/unit_tests/tools/test_compiler_wrapper.py b/tests/unit_tests/tools/test_compiler_wrapper.py index 11fdde57..93b26f14 100644 --- a/tests/unit_tests/tools/test_compiler_wrapper.py +++ b/tests/unit_tests/tools/test_compiler_wrapper.py @@ -7,7 +7,7 @@ '''Tests the compiler wrapper implementation. ''' -from pathlib import Path, PosixPath +from pathlib import Path from unittest import mock import pytest @@ -178,10 +178,10 @@ def test_compiler_wrapper_fortran_with_add_args(): syntax_only=True) # Notice that "-J/b" has been removed mpif90.compiler.run.assert_called_with( - cwd=PosixPath('.'), additional_parameters=['-c', "-O3", - '-fsyntax-only', - '-J', '/module_out', - 'a.f90', '-o', 'a.o']) + cwd=Path('.'), additional_parameters=['-c', "-O3", + '-fsyntax-only', + '-J', '/module_out', + 'a.f90', '-o', 'a.o']) def test_compiler_wrapper_fortran_with_add_args_unnecessary_openmp(): @@ -198,7 +198,7 @@ def test_compiler_wrapper_fortran_with_add_args_unnecessary_openmp(): add_flags=["-fopenmp", "-O3"], openmp=True, syntax_only=True) mpif90.compiler.run.assert_called_with( - cwd=PosixPath('.'), + cwd=Path('.'), additional_parameters=['-c', '-fopenmp', '-fopenmp', '-O3', '-fsyntax-only', '-J', '/module_out', 'a.f90', '-o', 'a.o']) @@ -218,8 +218,8 @@ def test_compiler_wrapper_c_with_add_args(): mpicc.compile_file(Path("a.f90"), "a.o", openmp=False, add_flags=["-O3"]) mpicc.compiler.run.assert_called_with( - cwd=PosixPath('.'), additional_parameters=['-c', "-O3", - 'a.f90', '-o', 'a.o']) + cwd=Path('.'), additional_parameters=['-c', "-O3", 'a.f90', + '-o', 'a.o']) # Invoke C compiler with syntax-only flag (which is only supported # by Fortran compilers), which should raise an exception. with pytest.raises(RuntimeError) as err: @@ -237,7 +237,7 @@ def test_compiler_wrapper_c_with_add_args(): add_flags=["-fopenmp", "-O3"], openmp=True) mpicc.compiler.run.assert_called_with( - cwd=PosixPath('.'), + cwd=Path('.'), additional_parameters=['-c', '-fopenmp', '-fopenmp', '-O3', 'a.f90', '-o', 'a.o']) @@ -280,7 +280,7 @@ def test_compiler_wrapper_flags_with_add_arg(): mpicc.compile_file(Path("a.f90"), "a.o", add_flags=["-f"], openmp=True) mpicc.compiler.run.assert_called_with( - cwd=PosixPath('.'), + cwd=Path('.'), additional_parameters=["-c", "-fopenmp", "-a", "-b", "-d", "-e", "-f", "a.f90", "-o", "a.o"]) @@ -299,7 +299,7 @@ def test_compiler_wrapper_flags_without_add_arg(): # Test if no add_flags are specified: mpicc.compile_file(Path("a.f90"), "a.o", openmp=True) mpicc.compiler.run.assert_called_with( - cwd=PosixPath('.'), + cwd=Path('.'), additional_parameters=["-c", "-fopenmp", "-a", "-b", "-d", "-e", "a.f90", "-o", "a.o"])