-
Notifications
You must be signed in to change notification settings - Fork 140
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
Use six instead of twisted.python.compat #290
Conversation
I'm guessing an empty newsfragment isn't needed here. If it is, let me know. |
src/treq/client.py
Outdated
@@ -377,12 +364,10 @@ def _guess_content_type(filename): | |||
registerAdapter(_from_bytes, bytes, IBodyProducer) | |||
registerAdapter(_from_file, BytesIO, IBodyProducer) | |||
|
|||
if not _PY3: | |||
from StringIO import StringIO | |||
if PY2: | |||
registerAdapter(_from_file, StringIO, IBodyProducer) |
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.
io.StringIO is the wrong StringIO
registerAdapter(_from_file, StringIO, IBodyProducer) | |
registerAdapter(_from_file, six.StringIO, IBodyProducer) |
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.
it's best to
import io
import six
and then use io.StringIO
or six.StringIO
so it's obvious
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.
I think it is coming from six
. (aren't these from
imports great?)
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.
ah right, that's super confusing
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.
@altendky the first hit for six
uses an idiomatic import:
treq/src/treq/test/local_httpbin/child.py
Line 13 in bea6878
import six |
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.
I'd recommend following modernize here and using idiomatic-style imports for both io
and six
https://modernize.readthedocs.io/en/latest/fixers.html#unicode_type
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.
Over following the existing local practice? (and that import is off in some other file that i didn't get to touching)
A newsfile along the lines of "Compatibility with Twisted 20.9.0" would be nice, except we don't know what version to put there. I will add that retroactively I guess. |
Thank you both for this! I will release once #284 is in. |
#288