Skip to content

Commit

Permalink
ORM: Fix typing of aiida.orm.nodes.data.code module
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sphuber committed Dec 8, 2022
1 parent a86e6fb commit 158d331
Show file tree
Hide file tree
Showing 8 changed files with 69 additions and 40 deletions.
3 changes: 0 additions & 3 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down
12 changes: 10 additions & 2 deletions aiida/orm/nodes/data/code/abstract.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion aiida/orm/nodes/data/code/containerized.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
30 changes: 25 additions & 5 deletions aiida/orm/nodes/data/code/installed.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -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:
Expand All @@ -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)
Expand Down
38 changes: 23 additions & 15 deletions aiida/orm/nodes/data/code/legacy.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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):
"""
Expand Down Expand Up @@ -140,35 +140,36 @@ 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):
"""Get full label of this code.
Returns label of the form <code-label>@<computer-name>.
"""
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.
: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

Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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:
Expand Down
14 changes: 8 additions & 6 deletions aiida/orm/nodes/data/code/portable.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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:
Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions tests/orm/data/code/test_abstract.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
6 changes: 0 additions & 6 deletions tests/orm/data/code/test_installed.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 158d331

Please sign in to comment.