Skip to content
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

Fixing integration with pytest-flake8 #1422

Closed
wants to merge 5 commits into from

Conversation

VRGhost
Copy link

@VRGhost VRGhost commented Oct 13, 2021

Hello,

The recent change to flake8's _stdoutWriteHook ( 7a353d2 ) breaks integration with some wrappers that attempt to capture flake8's sys.stdout output (e.g. pytest-flake8 does @ https://github.com/tholo/pytest-flake8/blob/master/pytest_flake8.py#L119 ), as it is uncommon for the capture code to emulate the .buffer attribute.

This PR modifies the stdout.write logic to be similar to sys.displayhook's ( https://docs.python.org/3/library/sys.html#displayhook ) that should keep everyone happy, hopefully - both Windows and integration people.

@asottile
Copy link
Member

if you monkeypatch, that is at your own risk -- use io.TextIOWrapper instead

@RonnyPfannschmidt
Copy link

@asottile this is pytest output capture, i believe this should reopen /fix

@asottile
Copy link
Member

@RonnyPfannschmidt it is not, pytest output capture does this correctly (I know because it was my first major contribution to pytest!)

def test():
    sys.stdout.buffer.write(b'hi\n')

@thijskramer
Copy link

But what (kind of) solution would both solve the problem that sys.stdout sometimes doesn't have a buffer attribute, and would be acceptable to you, @asottile ?

@RonnyPfannschmidt
Copy link

@asottile i see, ill crosscheck pytest flake8 once I recover from my cold as it seems to trigger this

@asottile
Copy link
Member

sys.stdout always has the .buffer attribute unless it is poorly monkeypatched. as I said above, you probably want io.TextIOWrapper when replacing sys.stdout to properly match the real sys.stdout

@ptrcnull
Copy link

sys.stdout always has the .buffer attribute unless it is poorly monkeypatched. as I said above, you probably want io.TextIOWrapper when replacing sys.stdout to properly match the real sys.stdout

it is not "poorly monkeypatched" - even the official Python 3 documentation warns about this kind of situation:
https://docs.python.org/3/library/sys.html#sys.stdout

However, if you are writing a library (and do not control in which context its code will be executed), be aware that the standard streams may be replaced with file-like objects like io.StringIO which do not support the buffer attribute.

flake8 in this case should just check if sys.stdout has the buffer property and act accordingly..

@asottile
Copy link
Member

the docs warn that people may do this wrong yes -- that doesn't mean we should support unsupported things

@PyCQA PyCQA locked as resolved and limited conversation to collaborators Jul 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants