Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make load_profile importable from aiida module #6609

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from
1 change: 1 addition & 0 deletions docs/source/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@ def setup(app: Sphinx):
'BackendLog': 'aiida.orm.implementation.logs.BackendLog',
'BackendNode': 'aiida.orm.implementation.nodes.BackendNode',
'BackendUser': 'aiida.orm.implementation.users.BackendUser',
'InteractiveShell': 'IPython.core.interactiveshell.InteractiveShell',
}


Expand Down
1 change: 1 addition & 0 deletions docs/source/nitpick-exceptions
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ py:class PGTest
py:class pgtest.pgtest.PGTest

py:class IPython.core.magic.Magics
py:class IPython.core.interactiveshell.InteractiveShell

py:class HTMLParser.HTMLParser
py:class html.parser.HTMLParser
Expand Down
15 changes: 12 additions & 3 deletions src/aiida/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,15 @@
More information at http://www.aiida.net
"""

from aiida.common.log import configure_logging # noqa: F401
from aiida.manage.configuration import get_config_option, get_profile, load_profile, profile_context # noqa: F401
from __future__ import annotations

from typing import TYPE_CHECKING

from aiida.common.log import configure_logging
from aiida.manage.configuration import get_config_option, get_profile, load_profile, profile_context

if TYPE_CHECKING:
from IPython.core.interactiveshell import InteractiveShell

Check warning on line 30 in src/aiida/__init__.py

View check run for this annotation

Codecov / codecov/patch

src/aiida/__init__.py#L30

Added line #L30 was not covered by tests

__copyright__ = (
'Copyright (c), This file is part of the AiiDA platform. '
Expand All @@ -35,6 +42,8 @@
)
__paper_short__ = 'S. P. Huber et al., Scientific Data 7, 300 (2020).'

__all__ = ['load_profile', 'configure_logging', 'get_config_option', 'get_profile', 'profile_context']
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes no sense to expose configure_logging to end user. But for backward compatibility, I keep it for the moment. I prefer to remove it, agree?

For load_profile and profile_context and get_config_option, they are mentioned in the documentation so I keep those. The get_profile also seems quite handy.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order for this to not be a breaking change, you'll have to include get_strict_version and load_ipython_extension as well.

Copy link
Member Author

@unkcpz unkcpz Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What you mean? It was not there, I think by adding things no change is breaking.
The user still can do:

import aiida
aiida.get_strict_version

Even without the function in the __all__.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but if they did from aiida import * than now suddenly get_strict_version will not be available.

Copy link
Collaborator

@danielhollas danielhollas Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In other words, I believe when __all__ is not defined, all symbols are imported when you do star import.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested, that's true, I didn't know before. Then I'll add them to __all__.



def get_strict_version():
"""Return a distutils StrictVersion instance with the current distribution version
Expand Down Expand Up @@ -93,7 +102,7 @@
return '\n'.join(f'{comment_char}{line}' for line in lines)


def load_ipython_extension(ipython):
def load_ipython_extension(ipython: InteractiveShell):
"""Load the AiiDA IPython extension, using ``%load_ext aiida``."""
from .tools.ipython.ipython_magics import AiiDALoaderMagics

Expand Down
2 changes: 1 addition & 1 deletion src/aiida/common/log.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ def evaluate_logging_configuration(dictionary):
return result


def configure_logging(with_orm=False, daemon=False, daemon_log_file=None):
def configure_logging(with_orm: bool = False, daemon: bool = False, daemon_log_file: None | str = None) -> None:
"""Setup the logging by retrieving the LOGGING dictionary from aiida and passing it to
the python module logging.config.dictConfig. If the logging needs to be setup for the
daemon, set the argument 'daemon' to True and specify the path to the log file. This
Expand Down
14 changes: 7 additions & 7 deletions src/aiida/engine/daemon/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,31 +156,31 @@

@property
def circus_log_file(self) -> str:
return self.profile.filepaths['circus']['log']
return str(self.profile.circus_log_file)

@property
def circus_pid_file(self) -> str:
return self.profile.filepaths['circus']['pid']
return str(self.profile.circus_pid_file)

@property
def circus_port_file(self) -> str:
return self.profile.filepaths['circus']['port']
return str(self.profile.circus_port_file)

Check warning on line 167 in src/aiida/engine/daemon/client.py

View check run for this annotation

Codecov / codecov/patch

src/aiida/engine/daemon/client.py#L167

Added line #L167 was not covered by tests

@property
def circus_socket_file(self) -> str:
return self.profile.filepaths['circus']['socket']['file']
return str(self.profile.circus_socket_endpoints['file'])

@property
def circus_socket_endpoints(self) -> dict[str, str]:
return self.profile.filepaths['circus']['socket']
return {k: str(v) for k, v in self.profile.circus_socket_endpoints.items()}

@property
def daemon_log_file(self) -> str:
return self.profile.filepaths['daemon']['log']
return str(self.profile.daemon_log_file)

@property
def daemon_pid_file(self) -> str:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this a breaking change if the return type is changed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it is python, I don't regard it as a "breaking change". The mypy might be unhappy if you already use types on downsteram plugins. But functionality side it won't break anything.

Copy link
Member Author

@unkcpz unkcpz Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I think I am not correct, the function return type changed actually.
Then my argument would be, this is the function I think should not exposed as public API.

In the end, we need a clear thoughts and go through again things and make API exposed very clear. One typical bad impl is for scheduler plugins, the API we exposed to allow user to implement new scheduler is methods start with underscore. But that is another story, happy to discuss next week in person.

return self.profile.filepaths['daemon']['pid']
return str(self.profile.daemon_pid_file)

Check warning on line 183 in src/aiida/engine/daemon/client.py

View check run for this annotation

Codecov / codecov/patch

src/aiida/engine/daemon/client.py#L183

Added line #L183 was not covered by tests

def get_circus_port(self) -> int:
"""Retrieve the port for the circus controller, which should be written to the circus port file.
Expand Down
43 changes: 43 additions & 0 deletions src/aiida/manage/configuration/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

from .options import parse_option

# from .settings import DAEMON_DIR, DAEMON_LOG_DIR

if TYPE_CHECKING:
from aiida.orm.implementation import StorageBackend

Expand Down Expand Up @@ -229,6 +231,47 @@

return pathlib.Path(os.path.expanduser(parts.path))

@property
def circus_log_file(self) -> pathlib.Path:
from .settings import DAEMON_LOG_DIR

return DAEMON_LOG_DIR / f'circus_{self.name}.log'

@property
def circus_pid_file(self) -> pathlib.Path:
from .settings import DAEMON_DIR

return DAEMON_DIR / f'circus_{self.name}.pid'

@property
def circus_port_file(self) -> pathlib.Path:
from .settings import DAEMON_DIR

Check warning on line 248 in src/aiida/manage/configuration/profile.py

View check run for this annotation

Codecov / codecov/patch

src/aiida/manage/configuration/profile.py#L248

Added line #L248 was not covered by tests

return DAEMON_DIR / f'circus-{self.name}.port'

Check warning on line 250 in src/aiida/manage/configuration/profile.py

View check run for this annotation

Codecov / codecov/patch

src/aiida/manage/configuration/profile.py#L250

Added line #L250 was not covered by tests

@property
def circus_socket_endpoints(self) -> dict[str, pathlib.Path]:
from .settings import DAEMON_DIR

return {
'file': DAEMON_DIR / f'circus-{self.name}.sockets',
'controller': pathlib.Path('circus.c.sock'),
'pubsub': pathlib.Path('circus.p.sock'),
'stats': pathlib.Path('circus.s.sock'),
}

@property
def daemon_log_file(self) -> pathlib.Path:
from .settings import DAEMON_LOG_DIR

return DAEMON_LOG_DIR / f'aiida-{self.name}.log'

@property
def daemon_pid_file(self) -> pathlib.Path:
from .settings import DAEMON_DIR

Check warning on line 271 in src/aiida/manage/configuration/profile.py

View check run for this annotation

Codecov / codecov/patch

src/aiida/manage/configuration/profile.py#L271

Added line #L271 was not covered by tests

return DAEMON_DIR / f'aiida-{self.name}.pid'

Check warning on line 273 in src/aiida/manage/configuration/profile.py

View check run for this annotation

Codecov / codecov/patch

src/aiida/manage/configuration/profile.py#L273

Added line #L273 was not covered by tests

@property
def filepaths(self):
"""Return the filepaths used by this profile.
Expand Down
1 change: 1 addition & 0 deletions src/aiida/tools/pytest_fixtures/daemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ def test(daemon_client):
from aiida.engine.daemon import get_daemon_client
from aiida.engine.daemon.client import DaemonNotRunningException, DaemonTimeoutException

print(aiida_profile)
daemon_client = get_daemon_client(aiida_profile.name)

try:
Expand Down
Loading