Skip to content

Conversation

@JornVernee
Copy link
Contributor

@JornVernee JornVernee commented Nov 29, 2018

Python 3.1 removes the builtin types from the types. module, so it's use in that regard is non-portable. This PR changes the use of types with builtin type names where appropriate.

Additionally, since basestring is no longer in Python 3, it's usage is exchanged for str. The difference is that basestring is a common super type for unicode and str, so this is technically more restrictive, but also portable. I don't think using str instead of basestring will matter in practice, since it's only being used in type checks with isinstance to check if something is a string, and almost all the code uses the str type, since it's the default for strings.

@dougxc dougxc self-assigned this Nov 30, 2018
Missing StringIO import

use str instead of bytes since that seems to have been the intent (even though the 2to3 mapping tool says to use bytes)
@JornVernee
Copy link
Contributor Author

JornVernee commented Nov 30, 2018

Just did a force push since I realized a small mistake I made.

The 2to3 tool maps types.StringType to bytes [1], so that's what I used initially, but given the context it should probably just have been str, so I'm now using that instead. They are the same object in python 2 any ways, so it should only matter for python 3 (which still passes gate with the change).

[1] : https://stackoverflow.com/a/3772091

@JornVernee
Copy link
Contributor Author

JornVernee commented Nov 30, 2018

Just as a heads up; This will probably conflict with #176 and #178 since they modify some of the same lines (I missed this earlier, sorry)

If you get those other 2 first I will update this PR to fix any conflicts that pop up.

@JornVernee JornVernee changed the title [Python 3] Remove use of types module and basestring [WIP] [Python 3] Remove use of types module and basestring Dec 1, 2018
@JornVernee
Copy link
Contributor Author

Needs some other changes as well, will update once the dependencies are done

@JornVernee JornVernee closed this Dec 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants