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

Only produce python3 warning with python3 warnings enabled #24

Merged
merged 1 commit into from Jan 25, 2019

Conversation

Projects
None yet
4 participants
@asottile
Copy link
Contributor

asottile commented Jan 23, 2019

As it is, this warning is quite difficult to avoid in a number of circumstances that would automatically be fixed in python3.x. As such, this warning is often not useful. Here’s a few examples where I’ve encountered this that are a bit annoying to fix:

  • flake8: takes commandline arguments (sys.argv is native strings (bytes py2, str py3)) which are passed to configparser
  • flake8: passes string literals to configparser (string literals are native strings)
  • entrypoints: passes os.path.join(sys.path entry, …) to configparser (sys.path is native strings)

These warnings only appear in python2.x and any sane library is already testing against python3.x where these would be errors.

As such, I propose either outright removing this warning, or (as this PR does) guarding it by sys.py3kwarning which is only True when running with python2 -3 ...

Only produce python3 warning with python3 warnings enabled
As it is, this warning is quite difficult to avoid in a number of circumstances that would automatically be fixed in python3.x. As such, this warning is often not useful. Here’s a few examples where I’ve encountered this that are a bit annoying to fix:

- flake8: takes commandline arguments (sys.argv is native strings (bytes py2, str py3)) which are passed to configparser
- flake8: passes string literals to configparser (string literals are native strings)
- entrypoints: passes os.path.join(sys.path entry, …) to configparser (sys.path is native strings)

‌

These warnings only appear in python2.x and any sane library is already testing against python3.x where these would be errors.

‌

As such, I propose either outright removing this warning, or (as this PR does) guarding it by `sys.py3kwarning` which is only `True` when running with `python2 -3 ...`

@jaraco jaraco merged commit c554de3 into jaraco:master Jan 25, 2019

@@ -684,7 +684,7 @@ def read(self, filenames, encoding=None):
Return list of successfully read files.
"""
if PY2 and isinstance(filenames, bytes):
if PY2 and isinstance(filenames, bytes) and sys.py3kwarning:

This comment has been minimized.

@nsoranzo

nsoranzo Jan 25, 2019

This additional guard should have been added inside the if, only for the warnings.warn().
By adding it here, filenames = [filenames] is not executed.

This is breaking flake8 (which depends on configparser under Python 2) for me, setup.cfg of my project seems to be ignored with this change, while it works normally if I remove it.

I see that this code has just been removed in the master branch, are you planning to release configparser 3.7.0 soon?

This comment has been minimized.

@hashar

This comment has been minimized.

@jaraco

jaraco Jan 25, 2019

Owner

are you planning to release configparser 3.7.0 soon?

Yes. I actually thought I had. Indeed, I'd tagged it, just not pushed the releases. I'll do that now.

This comment has been minimized.

@nsoranzo

nsoranzo Jan 25, 2019

Thanks! Both 3.5.3 and 3.7.0 fix the issue for me.

@asottile asottile deleted the asottile:patch-2 branch Jan 25, 2019

@asottile

This comment has been minimized.

Copy link
Contributor Author

asottile commented Jan 26, 2019

Apologies on this, I should have been more careful :(

Thanks for fixing this!

@jaraco

This comment has been minimized.

Copy link
Owner

jaraco commented Jan 26, 2019

I too could have been more careful. I put too much trust in the test suite and the age of the (original) pull request. I found a broader lesson in the whole ordeal, though, which you may appreciate.

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