-
Notifications
You must be signed in to change notification settings - Fork 115
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 poutput to use new style_output definition. #1269
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #1269 +/- ##
==========================================
- Coverage 97.96% 97.93% -0.04%
==========================================
Files 22 22
Lines 5700 5705 +5
==========================================
+ Hits 5584 5587 +3
- Misses 116 118 +2
☔ View full report in Codecov by Sentry. |
I like the idea of adding a
@tleonhardt What are your thoughts? |
7ce6ed2
to
d5508e9
Compare
I feel part of the issue is our current print methods conflate where to send output with how to send output. I've made some adjustments that move in the direction of keeping things separate. |
…that uses style_success
…rom what and how to print
d5508e9
to
6d350e3
Compare
|
||
def pwarning(self, msg: Any = '', *, end: str = '\n', apply_style: bool = True) -> None: | ||
def pwarning( |
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.
Should the signature for pwarning()
match that of pfailure()
(i.e. remove apply_style
).
This seems appropriate since we now have two wrappers for perror()
. They should work the same way.
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 was reluctant to make an API breaking change to pwarning(). I don't know if it's possible to mark a single argument deprecated.
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 added a deprecation to the parameter documentation. We can remove it in the major version revision.
Closes #1268