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.

Sign up for GitHub

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

Remove get_description() from interface #59

Merged
merged 2 commits into from
Sep 7, 2024
Merged

Remove get_description() from interface #59

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8bcf167
removed get description from interface
khsrali Aug 20, 2024
c65d997
Merge branch 'main' into get_description
khsrali Aug 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 3 additions & 9 deletions aiida_firecrest/scheduler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@


class FirecrestScheduler(Scheduler):
"""Scheduler interface for FirecREST."""
"""Scheduler interface for FirecREST.
It must be used together with the 'firecrest' transport plugin.
"""

transport: FirecrestTransport
_job_resource_class = SlurmJobResource
Expand All @@ -32,14 +34,6 @@ class FirecrestScheduler(Scheduler):
_logger = Scheduler._logger.getChild("firecrest")
_DEFAULT_PAGE_SIZE = 25

@classmethod
def get_description(cls) -> str:
"""Used by verdi to describe the plugin."""
return (
"A plugin to connect to a FirecREST server.\n"
"It must be used together with the 'firecrest' transport plugin.\n"
)

def _get_submit_script_header(self, job_tmpl: JobTemplate) -> str:
"""
Return the submit script header, using the parameters from the
Expand Down
14 changes: 2 additions & 12 deletions aiida_firecrest/transport.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,8 @@ def _dynamic_info_direct_size(


class FirecrestTransport(Transport):
"""Transport interface for FirecREST."""
"""Transport interface for FirecREST.
It must be used together with the 'firecrest' scheduler plugin."""
Comment on lines +236 to +237
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you considered overwriting class variable __doc__? Then you can in principle also include this if you can retrieve auth_options is from another class

class AuthoptionHolder:
    auth_options = {"machine": "the name of the machine"}

class A:
    __doc__ = ( """Transport interface for FirecREST.
    It must be used together with the 'firecrest' scheduler plugin."""
    +
    "Authentication parameters:\n"
        ) + "\n".join(
            [f"  {k}: {v}" for k, v in AuthoptionHolder.auth_options.items()]
        )

A.__doc__

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @agoscinski , it's interesting.
But it doesn't work, because auth_options should be from the current class cls, not the one we inherit from.
So it easily turns into some fuss: one has to assign put the __doc__ after _valid_auth_options, and do some list/tuple/dict magic to take the info out.

Probably it's not that important. I don't thing if many people would like to see inputs of a class through verdi plugin list anyways.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the description is not so important, but class documentation I think is important, since you get it with help(<CLASS_NAME>). But my solution also complexifies code logic so I am ok to not do it.


# override these options, because they don't really make sense for a REST-API,
# so we don't want the user having to provide them
Expand Down Expand Up @@ -421,17 +422,6 @@ def payoff_override(self, value: bool) -> None:
raise ValueError("payoff_override must be a boolean value")
self._payoff_override = value

@classmethod
def get_description(cls) -> str:
"""Used by verdi to describe the plugin."""
return (
"A plugin to connect to a FirecREST server.\n"
"It must be used together with the 'firecrest' scheduler plugin.\n"
"Authentication parameters:\n"
) + "\n".join(
[f" {k}: {v.get('help', '')}" for k, v in cls.auth_options.items()]
)

def open(self) -> None:
"""Open the transport.
This is a no-op for the REST-API, as there is no connection to open.
Expand Down
Loading