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
Updated message from config check of unconnected inputs #1095
Conversation
naylor-b
commented
Oct 30, 2019
- now includes units and values
- any cases where the values for connected inputs don't match are highlighted
template_prom.format(a, u, | ||
_trim_str(problem.get_val(a, | ||
get_remote=True), | ||
25), |
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.
This is really hard to read. Could you create a temporary variable to hold _trim_str(problem.get_val(a,get_remote=True)
to improve readability? Added advantage, looks like you can do it outside of the if branch.
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.
fixed
a = absnames[0] | ||
msg.append(" " + | ||
template_abs.format(a, units[0], | ||
_trim_str(problem.get_val(a, get_remote=True), |
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.
Wonder if we should return the norm instead of the full array for large arrays. Maybe optionally.
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.
Which is the better default, norm or truncated value? We can't pass options to specific checks via the command line. We could make it an option to the internal check function I suppose, but that won't help with command line usage since it would require the user to use some complicated syntax.
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 know, maybe we should do both truncated and norm...
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 I updated it to use the norm if the string exceeds the specified width.