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

Remove Python 2 support and dependency on py, fixes #81 #82

Merged
merged 2 commits into from
Mar 5, 2022

Conversation

erikkemperman
Copy link
Contributor

Hi,

I just ran into #81 and looked into it a bit.

It seems that the stderr/stdout capturing, currently handled by py, doesn't agree with flake8 report generation...

Considering that py is in maintenance mode and Python 2 is officially pushing the daisies, I thought you might consider this PR which uses capturing utilities available in the standard library, contextlib -- since 3.4.

@erikkemperman
Copy link
Contributor Author

Sorry, docs say that redirecting stdout was added in 3.4, but stderr was only added in 3.5 -- amended the commit, and threw in 3.10 for good measure.

@justinrmiller
Copy link

Could we get this merged down and released? This is causing issues for us as well. Thank you!

siddharthvipul pushed a commit to CentOS/duffy that referenced this pull request Oct 27, 2021
Pytest-flake8 (by way of py.io) monkey-patches stdout, stderr in an
unclean way which is incompatible with how flake8 >= 4.0.0 uses them.

A fix for this has been submitted to pytest-flake8:

tholo/pytest-flake8#82

Until this or an equivalent change is available, we'll pin flake8 to a
version which works without the fix.

Signed-off-by: Nils Philippsen <[email protected]>
@khink
Copy link

khink commented Nov 16, 2021

This looks like a good fix, well at least it seems to agree with the pytest maintainer's suggestion in PyCQA/flake8#1422 ("use io.TextIOWrapper").

@tholo Do you have any reservations about this MR? Maybe some changes needed before it can be merged? People could help in getting these changes in, just let us know.

Copy link

@taliastocks taliastocks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it.

@boidolr
Copy link

boidolr commented Dec 21, 2021

Would love this too 👍
@erikkemperman I think a change to .travis.yml is needed too, to reflect the versions in the tests.

@erikkemperman
Copy link
Contributor Author

@boidolr Thanks, I've amended the travis config now.

@erikkemperman
Copy link
Contributor Author

Sorry for the force pushes, had not configured my gpg key properly on new laptop

@JulienPalard
Copy link

I tested this branch and it works for me.

modernNeo added a commit to CSSS/wall_e that referenced this pull request Jan 12, 2022
modernNeo added a commit to CSSS/wall_e that referenced this pull request Jan 12, 2022
@tholo tholo merged commit ec37880 into tholo:master Mar 5, 2022
tholo added a commit that referenced this pull request Mar 5, 2022
Removes Python 2 support as well as dependency on py; fixes #81 #82
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants