-
Notifications
You must be signed in to change notification settings - Fork 56
chore: Remove 'get_specs' from datamodel help. #4574
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
base: main
Are you sure you want to change the base?
Conversation
…github.com/ansys/pyfluent into maint/remove_get_specs_from_datamodel_help
| help_lines.extend( | ||
| [" " + line for line in param._get_help_string().splitlines()] | ||
| ) | ||
| elif isinstance(self, PyParameter): |
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.
Why not just override it?
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.
yes, will do.
| cls_name = self.__class__.__name__ | ||
| is_task_object = cls_name == "_TaskObject" | ||
|
|
||
| if is_task_object: |
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.
Related to the PyParameter comment below, this should also be handled polymorphically, or at least externally (a type-based strategy).
| name = str(self).split()[0].split(".")[-1] | ||
| default_val = getattr(self, "default_value", None) | ||
| default = default_val() if callable(default_val) else None | ||
| if isinstance(self, PyTextual): |
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.
Each of these conditions can be handled via an override per type.
| default_val = getattr(self, "default_value", None) | ||
| default = default_val() if callable(default_val) else None |
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 don't get what's going on here. Can we please have some comments?
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.
ok, sure. If the default value returns something then we add it. Will add some inline comments.
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.
In this case, "default_val" can either be the attr or None. In the next line, it checks if it is not None then return the value of "default_val". Will simplify the logic as well.
Context
'get_specs' was used for generating the dynamic help.
Change Summary
Dynamic help does not depend on 'get_specs' any longer.
Rationale
Dynamic help is constructed via attribute query as required.
Impact
Mostly no impact on the end users as the help text is expected to remain same.
PyCommand:
PyCommandArguments: