-
Notifications
You must be signed in to change notification settings - Fork 3
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
Remove get_description()
from interface
#59
Conversation
get_description()
from interface
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #59 +/- ##
==========================================
+ Coverage 79.32% 79.43% +0.11%
==========================================
Files 4 4
Lines 677 671 -6
==========================================
- Hits 537 533 -4
+ Misses 140 138 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
"""Transport interface for FirecREST. | ||
It must be used together with the 'firecrest' scheduler plugin.""" |
There was a problem hiding this comment.
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__
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
thanks for the review, @agoscinski |
Fixes:
#58