-
Notifications
You must be signed in to change notification settings - Fork 7
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
IPython "WARNING" log level #27
Conversation
@andrewmkiss, did it work for you? |
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 looks like a good idea. I did like the way we saw the startup scripts running at beamlines that do not already print that to the console, so I am reluctant to apply it everywhere. On the other hand we probably don't want a high level of ipython logging.
Thanks for your review, @jklynch! What's interesting, this change only affects the messages printed to the terminal. I didn't see the log entries in the |
I had to think about it for a while. I guess we are currently sending only ipython console input, output, and exceptions to |
Yep, that's what I've noticed too. |
Sorry, we have been busy with the beamline. I can try to check this on Friday. |
I want to emphasize that I think at beamlines that do not explicitly print startup script names the current behavior is valuable and I would like to keep it. I have not noticed other IPython logging messages but if someone has an example I would be interested to see it. Also since these messages are coming from the usual python logging framework it should be possible to send them to the bluesky ipython log file, which would make me happy. |
The IPython I'll rebase on |
ebc8b39
to
5ce0ef0
Compare
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.
Looking into here, it seems like the fix was already added to the master
branch. Not sure how much we need from this PR then. Just new lines? 😄
Yes, I think I had added the one or two lines of code to "test" before merging and those lines of code were eventually absorbed into master. I can approve the changes. Do we still need to merge? |
Thanks for the re-review, Andy. Let's merge to have a slightly better formatting :) |
An alternative fix/workaround of NSLS-II/nslsii#115. I tested it, and now it does not print the
[TerminalIPythonApp]
messages to the screen. It would be still nice to have it configurable innslsii
.