Skip to content

Bpython displays logging on single line in Python 3.4 & 3.5 #658

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

Closed
xlash opened this issue Nov 30, 2016 · 12 comments
Closed

Bpython displays logging on single line in Python 3.4 & 3.5 #658

xlash opened this issue Nov 30, 2016 · 12 comments

Comments

@xlash
Copy link

xlash commented Nov 30, 2016

The following is not displayed properly in Python 3.4.2, and 3.5.2. Is successfull on 2.7.12. Only happens in a loop scenario, but it's not a flushing timing.

✗ bpython             
bpython version 0.16 on top of Python 3.4.2 /home/gnm/PyEnvs/MSS_Reports/bin/python3.4
>>> import logging
>>> for i in range(0,10,1):
...     logging.critical('test')
...     
... 










CRITICAL:root:testCRITICAL:root:testCRITICAL:root:testCRITICAL:root:testCRITICAL:root:testCRITICAL:root:testCRITICAL:root:testCRITICAL:root:testCR
ITICAL:root:testCRITICAL:root:test
>>> for i in range(0,10,1):
...     print('test')
...     
... 
test
test
test
test
test
test
test
test
test
test

This is working also correctly in the normal python interpreter.

Thanks for help.

@thomasballinger
Copy link
Member

Huh, interesting. Thanks for submitting @xlash.

For anyone looking for things to investigate, here are some questions I'm wondering:

  • We do some funny newline handling things in the code runner. Does our code implement different behavior in 2 vs 3?
  • What calls exactly are made by the default log handler? How are these different in 2 vs 3?

One guess: the log handler is batching things more in Python 3?

@ba11b0y
Copy link

ba11b0y commented Jan 28, 2017

try this:
``

import logging
for i in range(0,10,1):
... logging.critical("test\n")
``

@ba11b0y
Copy link

ba11b0y commented Jan 28, 2017

I am very much interested , do help me if I am wrong.

@xlash
Copy link
Author

xlash commented Jan 28, 2017

This is against the purpose of 1-liner log. On Python <3.5, it will gives this :
CRITICAL:root:test

CRITICAL:root:test

CRITICAL:root:test

CRITICAL:root:test

CRITICAL:root:test

CRITICAL:root:test

CRITICAL:root:test

CRITICAL:root:test

CRITICAL:root:test

CRITICAL:root:test

(Notice the space in between). This is not a valid work around.

@kingtong
Copy link

kingtong commented Apr 3, 2017

StreamHandler in logging is actually performing two writes when emitting a record.
From Python 3.6.1, logging/init.py, line 994

stream.write(msg)
stream.write(self.terminator)

Whereas in python 2.7.13 it was in the same context as above:

fs = "%s\n"
if not _unicode: #if no unicode support...
    stream.write(fs % msg) 

So I think the issue is not related to logging but in the handling of two successive write:

➜  bpython           
bpython version 0.16 on top of Python 3.6.1 /home/nicolas/.pyenv/versions/3.6.1/envs/python3.6/bin/python
>>> import sys
>>> for i in range(2):
...     sys.stderr.write('test')
...     sys.stderr.write('\n')
...     
... 


testtest

I will continue to dive into bpython's code but I am new to it, so if anyone has a fix or a way to handle this...

@thomasballinger
Copy link
Member

thomasballinger commented Apr 4, 2017

Thanks for taking a look @kingtong. Here's some context on the code around where bpython does this.

bpython replaces sys.stdout and sys.stderr with our own versions to intercept these output streams so we can format them ourselves. Some things we do with them:

  • parse out terminal formatting sequences so we can reapply our own version
  • strip other formatting characters that we can't currently imitate (\r for example, but lots of others)

Each time a write to one of these occurs, we add that new data to our model of the terminal screen and repaint the current view of the REPL (React declarative view style). There's an attribute on the bpython.curtsiesfrontend.repl.BaseRepl called current_stdouterr_line which is where a stderr.write( should end up adding to, via methods like send_to_stderr.

From a cursory look, I think sends_to_stderr is being used in two places: the interpreter uses it for writing tracebacks and the sys.stderr fake output object uses it for normal writes to stderr. It could be that it does things that are really only appropriate for one of these or the other, and that there should really be two different methods.

Somewhere in this path that output takes to be written there's a heuristic for splitting stderr lines that isn't accurate.

Another place to look for relevant code is in bpython/curtsiesfrontend/coderunner.py, and in the FakeStdout back in bpython/curtsiesfrontend/repl.

@thomasballinger
Copy link
Member

note to myself to look at this in #681 since I'll be poking at this logic anyway

@ata2001
Copy link
Contributor

ata2001 commented May 19, 2017

There's a logic in the run_code_and_maybe_finish method of BaseRepl class (curtsies/repl.py, line 1084), what moves the current_stdouterr_line to display_lines and cleans it.

The problem is, that it would need to be ran every time something writes to the stderr, not only once in a loop.

This issue could be solved by doing it in send_to_stderr instead.

Please wait, till I get home! I'm going to create a PR.

Edit: I've just realized, when writing to stderr, it should not start a new line on every write.

@ata2001
Copy link
Contributor

ata2001 commented May 26, 2017

I'm still working on this, but my solution uses the splitlines method, which is not working on FmtStrs right now.
It's blocked on https://github.com/thomasballinger/curtsies/issues/101.

@ata2001
Copy link
Contributor

ata2001 commented May 27, 2017

Didn't need splitlines, created PR: #686.
Please review!

@sebastinas
Copy link
Contributor

Fixed in 724f3b9

@thomasballinger
Copy link
Member

Great work @ata2001!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants