From 158d33145317326c8eefbfe369c801eb8e88d79a Mon Sep 17 00:00:00 2001 From: Sebastiaan Huber Date: Thu, 8 Dec 2022 14:28:28 +0100 Subject: [PATCH] ORM: Fix typing of `aiida.orm.nodes.data.code` module The original type `PurePath` being returned by `filepath_executable` and `get_execname` has been changed to `PurePosixPath` since currently the rest of AiiDA's infrastructure requires/assumes that `Computer`s are running POSIX operating systems. --- .pre-commit-config.yaml | 3 -- aiida/orm/nodes/data/code/abstract.py | 12 +++++-- aiida/orm/nodes/data/code/containerized.py | 2 +- aiida/orm/nodes/data/code/installed.py | 30 ++++++++++++++--- aiida/orm/nodes/data/code/legacy.py | 38 +++++++++++++--------- aiida/orm/nodes/data/code/portable.py | 14 ++++---- tests/orm/data/code/test_abstract.py | 4 +-- tests/orm/data/code/test_installed.py | 6 ---- 8 files changed, 69 insertions(+), 40 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 897bed9e43..a542a72902 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -128,9 +128,6 @@ repos: aiida/orm/nodes/data/array/bands.py| aiida/orm/nodes/data/array/trajectory.py| aiida/orm/nodes/data/cif.py| - aiida/orm/nodes/data/code/installed.py| - aiida/orm/nodes/data/code/legacy.py| - aiida/orm/nodes/data/code/portable.py| aiida/orm/nodes/data/remote/base.py| aiida/orm/nodes/data/structure.py| aiida/orm/nodes/data/upf.py| diff --git a/aiida/orm/nodes/data/code/abstract.py b/aiida/orm/nodes/data/code/abstract.py index aab68e4def..ed87a4f0a7 100644 --- a/aiida/orm/nodes/data/code/abstract.py +++ b/aiida/orm/nodes/data/code/abstract.py @@ -76,7 +76,7 @@ def can_run_on_computer(self, computer: Computer) -> bool: """ @abc.abstractmethod - def get_executable(self) -> pathlib.Path: + def get_executable(self) -> pathlib.PurePosixPath: """Return the executable that the submission script should execute to run the code. :return: The executable to be called in the submission script. @@ -129,7 +129,15 @@ def full_label(self) -> str: :return: The full label of the code. """ - @Data.label.setter # type: ignore + @property + def label(self) -> str: + """Return the label. + + :return: The label. + """ + return self.backend_entity.label + + @label.setter def label(self, value: str) -> None: """Set the label. diff --git a/aiida/orm/nodes/data/code/containerized.py b/aiida/orm/nodes/data/code/containerized.py index c18535ba68..6704c884cb 100644 --- a/aiida/orm/nodes/data/code/containerized.py +++ b/aiida/orm/nodes/data/code/containerized.py @@ -36,7 +36,7 @@ def __init__(self, engine_command: str, image_name: str, **kwargs): self.image_name = image_name @property - def filepath_executable(self) -> pathlib.PurePath: + def filepath_executable(self) -> pathlib.PurePosixPath: """Return the filepath of the executable that this code represents. .. note:: This is overridden from the base class since the path does not have to be absolute. diff --git a/aiida/orm/nodes/data/code/installed.py b/aiida/orm/nodes/data/code/installed.py index 957888a24e..008f867e71 100644 --- a/aiida/orm/nodes/data/code/installed.py +++ b/aiida/orm/nodes/data/code/installed.py @@ -14,6 +14,8 @@ represented by an instance of :class:`aiida.orm.computers.Computer`. Each time a :class:`aiida.engine.CalcJob` is run using an ``InstalledCode``, it will run its executable on the associated computer. """ +from __future__ import annotations + import pathlib import click @@ -42,7 +44,7 @@ def __init__(self, computer: Computer, filepath_executable: str, **kwargs): """ super().__init__(**kwargs) self.computer = computer - self.filepath_executable = filepath_executable + self.filepath_executable = filepath_executable # type: ignore[assignment] def _validate(self): """Validate the instance by checking that a computer has been defined. @@ -92,13 +94,31 @@ def can_run_on_computer(self, computer: Computer) -> bool: type_check(computer, Computer) return computer.pk == self.computer.pk - def get_executable(self) -> pathlib.Path: + def get_executable(self) -> pathlib.PurePosixPath: """Return the executable that the submission script should execute to run the code. :return: The executable to be called in the submission script. """ return self.filepath_executable + @property # type: ignore[override] + def computer(self) -> Computer: + """Return the computer of this code.""" + assert self.backend_entity.computer is not None + return Computer.from_backend_entity(self.backend_entity.computer) + + @computer.setter + def computer(self, computer: Computer) -> None: + """Set the computer of this code. + + :param computer: A `Computer`. + """ + if self.is_stored: + raise exceptions.ModificationNotAllowed('cannot set the computer on a stored node') + + type_check(computer, Computer, allow_none=False) + self.backend_entity.computer = computer.backend_entity + @property def full_label(self) -> str: """Return the full label of this code. @@ -111,12 +131,12 @@ def full_label(self) -> str: return f'{self.label}@{self.computer.label}' @property - def filepath_executable(self) -> pathlib.PurePath: + def filepath_executable(self) -> pathlib.PurePosixPath: """Return the absolute filepath of the executable that this code represents. :return: The absolute filepath of the executable. """ - return pathlib.PurePath(self.base.attributes.get(self._KEY_ATTRIBUTE_FILEPATH_EXECUTABLE)) + return pathlib.PurePosixPath(self.base.attributes.get(self._KEY_ATTRIBUTE_FILEPATH_EXECUTABLE)) @filepath_executable.setter def filepath_executable(self, value: str) -> None: @@ -126,7 +146,7 @@ def filepath_executable(self, value: str) -> None: """ type_check(value, str) - if not pathlib.PurePath(value).is_absolute(): + if not pathlib.PurePosixPath(value).is_absolute(): raise ValueError('the `filepath_executable` should be absolute.') self.base.attributes.set(self._KEY_ATTRIBUTE_FILEPATH_EXECUTABLE, value) diff --git a/aiida/orm/nodes/data/code/legacy.py b/aiida/orm/nodes/data/code/legacy.py index 28abc59ac2..9505dd4326 100644 --- a/aiida/orm/nodes/data/code/legacy.py +++ b/aiida/orm/nodes/data/code/legacy.py @@ -86,7 +86,7 @@ def can_run_on_computer(self, computer: Computer) -> bool: type_check(computer, orm.Computer) return computer.pk == self.get_remote_computer().pk - def get_executable(self) -> pathlib.Path: + def get_executable(self) -> pathlib.PurePosixPath: """Return the executable that the submission script should execute to run the code. :return: The executable to be called in the submission script. @@ -96,7 +96,7 @@ def get_executable(self) -> pathlib.Path: else: exec_path = self.get_remote_exec_path() - return pathlib.Path(exec_path) + return pathlib.PurePosixPath(exec_path) def hide(self): """ @@ -140,16 +140,17 @@ def set_files(self, files): self.base.repository.put_object_from_filelike(handle, os.path.split(filename)[1]) def __str__(self): - local_str = 'Local' if self.is_local() else 'Remote' - computer_str = self.computer.label - return f"{local_str} code '{self.label}' on {computer_str}, pk: {self.pk}, uuid: {self.uuid}" + if self.computer is None: + return f"Local code '{self.label}' pk: {self.pk}, uuid: {self.uuid}" + + return f"Remote code '{self.label}' on {self.computer.label} pk: {self.pk}, uuid: {self.uuid}" def get_computer_label(self): """Get label of this code's computer.""" warn_deprecation( 'This method is deprecated, use the `InstalledCode.computer.label` property instead.', version=3 ) - return 'repository' if self.is_local() else self.computer.label + return 'repository' if self.computer is None else self.computer.label @property def full_label(self): @@ -157,7 +158,7 @@ def full_label(self): Returns label of the form @. """ - return f'{self.label}@{"repository" if self.is_local() else self.computer.label}' + return f'{self.label}@{"repository" if self.computer is None else self.computer.label}' def relabel(self, new_label): """Relabel this code. @@ -165,10 +166,10 @@ def relabel(self, new_label): :param new_label: new code label """ warn_deprecation('This method is deprecated, use the `label` property instead.', version=3) - # pylint: disable=unused-argument - suffix = f'@{self.computer.label}' - if new_label.endswith(suffix): - new_label = new_label[:-len(suffix)] + if self.computer is not None: + suffix = f'@{self.computer.label}' + if new_label.endswith(suffix): + new_label = new_label[:-len(suffix)] self.label = new_label @@ -205,11 +206,15 @@ def get_code_helper(cls, label, machinename=None, backend=None): elif query.count() > 1: codes = query.all(flat=True) retstr = f"There are multiple codes with label '{label}', having IDs: " - retstr += f"{', '.join(sorted([str(c.pk) for c in codes]))}.\n" + retstr += f"{', '.join(sorted([str(c.pk) for c in codes]))}.\n" # type: ignore[union-attr] retstr += ('Relabel them (using their ID), or refer to them with their ID.') raise MultipleObjectsError(retstr) else: - return query.first()[0] + result = query.first() + if not result: + raise NotExistent(f"code '{label}' does not exist.") + + return result[0] @classmethod def get(cls, pk=None, label=None, machinename=None): @@ -303,9 +308,9 @@ def list_for_plugin(cls, plugin, labels=True, backend=None): valid_codes = query.all(flat=True) if labels: - return [c.label for c in valid_codes] + return [c.label for c in valid_codes] # type: ignore[union-attr] - return [c.pk for c in valid_codes] + return [c.pk for c in valid_codes] # type: ignore[union-attr] def _validate(self): super()._validate() @@ -347,6 +352,9 @@ def validate_remote_exec_path(self): ) filepath = self.get_remote_exec_path() + if self.computer is None: + raise exceptions.ValidationError('checking the remote exec path is not available for a local code.') + try: with override_log_level(): # Temporarily suppress noisy logging with self.computer.get_transport() as transport: diff --git a/aiida/orm/nodes/data/code/portable.py b/aiida/orm/nodes/data/code/portable.py index 955f0fc42b..8fb2bd8364 100644 --- a/aiida/orm/nodes/data/code/portable.py +++ b/aiida/orm/nodes/data/code/portable.py @@ -17,6 +17,8 @@ :class:`aiida.engine.CalcJob` is run using a ``PortableCode``, the uploaded files will be automatically copied to the working directory on the selected computer and the executable will be run there. """ +from __future__ import annotations + import pathlib import click @@ -54,8 +56,8 @@ def __init__(self, filepath_executable: str, filepath_files: pathlib.Path, **kwa """ super().__init__(**kwargs) type_check(filepath_files, pathlib.Path) - self.filepath_executable = filepath_executable - self.base.repository.put_object_from_tree(filepath_files) + self.filepath_executable = filepath_executable # type: ignore[assignment] + self.base.repository.put_object_from_tree(str(filepath_files)) def _validate(self): """Validate the instance by checking that an executable is defined and it is part of the repository files. @@ -86,7 +88,7 @@ def can_run_on_computer(self, computer: Computer) -> bool: """ return True - def get_executable(self) -> pathlib.Path: + def get_executable(self) -> pathlib.PurePosixPath: """Return the executable that the submission script should execute to run the code. :return: The executable to be called in the submission script. @@ -123,12 +125,12 @@ def full_label(self) -> str: return self.label @property - def filepath_executable(self) -> pathlib.PurePath: + def filepath_executable(self) -> pathlib.PurePosixPath: """Return the relative filepath of the executable that this code represents. :return: The relative filepath of the executable. """ - return pathlib.PurePath(self.base.attributes.get(self._KEY_ATTRIBUTE_FILEPATH_EXECUTABLE)) + return pathlib.PurePosixPath(self.base.attributes.get(self._KEY_ATTRIBUTE_FILEPATH_EXECUTABLE)) @filepath_executable.setter def filepath_executable(self, value: str) -> None: @@ -138,7 +140,7 @@ def filepath_executable(self, value: str) -> None: """ type_check(value, str) - if pathlib.PurePath(value).is_absolute(): + if pathlib.PurePosixPath(value).is_absolute(): raise ValueError('The `filepath_executable` should not be absolute.') self.base.attributes.set(self._KEY_ATTRIBUTE_FILEPATH_EXECUTABLE, value) diff --git a/tests/orm/data/code/test_abstract.py b/tests/orm/data/code/test_abstract.py index e2eef3c203..7047401202 100644 --- a/tests/orm/data/code/test_abstract.py +++ b/tests/orm/data/code/test_abstract.py @@ -23,9 +23,9 @@ def can_run_on_computer(self, computer) -> bool: """Return whether the code can run on a given computer.""" return True - def get_executable(self) -> pathlib.Path: + def get_executable(self) -> pathlib.PurePosixPath: """Return the executable that the submission script should execute to run the code.""" - return pathlib.Path('/bin/executable') + return pathlib.PurePosixPath('/bin/executable') @property def full_label(self) -> str: diff --git a/tests/orm/data/code/test_installed.py b/tests/orm/data/code/test_installed.py index 91fcec2695..af1a46eb9d 100644 --- a/tests/orm/data/code/test_installed.py +++ b/tests/orm/data/code/test_installed.py @@ -43,12 +43,6 @@ def test_validate(aiida_localhost): filepath_executable = '/usr/bin/bash' code = InstalledCode(computer=aiida_localhost, filepath_executable=filepath_executable) - code.computer = None - assert code.computer is None - - with pytest.raises(ValidationError, match='The `computer` is undefined.'): - code.store() - code.computer = aiida_localhost code.base.attributes.set(code._KEY_ATTRIBUTE_FILEPATH_EXECUTABLE, None) # pylint: disable=protected-access