-
Notifications
You must be signed in to change notification settings - Fork 330
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
ENH: #448 print flush #527
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #527 +/- ##
==========================================
+ Coverage 94.05% 94.31% +0.25%
==========================================
Files 38 37 -1
Lines 4157 4099 -58
==========================================
- Hits 3910 3866 -44
+ Misses 247 233 -14 ☔ View full report in Codecov by Sentry. |
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.
Good job improving the tests and the conditional! In the issue, I recommended to lookup what other kwargs print might accept, too. Do you think sep
, end
, or file
are useful?
IDK if they are, but it it wouldn't be hard to implement them all. Are there any reasons not to use **kwargs
?
def print(self, lhs=None, precision=3, **kwargs):
And just passing the kwargs to the builtin print with
print(f"{names} = {eqn}", **kwargs)
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, Watcharin, just a few deletions and we're good to go
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.
Hey, BTW, I meant delete the whole test test_print_discrete_time_multiple_trajectories, rather than just a single line. Could you do that please?
#448 add flush options to model.print