-
Notifications
You must be signed in to change notification settings - Fork 134
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
Python 3 Support #81
Python 3 Support #81
Conversation
@michaeltryby, I'm finally coming out of the stone ages ;-) |
@bemcdonnell why 3.4? we should aim for either 3.5 or 3.6 |
The changes you made to utf8 encode are not robust enough if the plan is to support both python2.7 and 3. Take a look at six library and use that please. |
@goanpeca, I updated the encoding behavior to use the python six module. Also, testing for 2.7, 3.4, 3.5, and 3.6 are all a pass! Let me know if you approve of these changes. |
Hey @bemcdonnell looking much better Still not sure about the |
if self._cuindex < self._nLinks: | ||
linkobject = Link(self._model, self._linkid) | ||
self._cuindex += 1 # Next Iteration | ||
return linkobject | ||
else: | ||
raise StopIteration() | ||
|
||
next = __next__ # Python 2 |
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.
Why do we need this?
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.
So as I understand, the method __next__
was day lighted in Python 3 pep3114. In py2, by default, the iterator calls next()
therefore, I had to add this line in order for it to step.
I could use the 2to3
package but that doesn't feel like progress to me. If you know of a better way, please enlighten me!
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.
six.next(<iterator>)
?
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 mean remove those redefinitions and use directly six.next
does not that work?
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'm not having any success with your suggestion. This code porting guide shows the iterator next
implementation as I have it above: Conservative Python 3 Porting Guide (see the end). Maybe there is still a better way?
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.
Ok yes you are right :-)
@bemcdonnell we need to update the dependencies on setup.py to include six now. |
@goanpeca, shall I merge? |
yeah ;-) |
I've got a first iteration here for Py34 support on Windows. If you guys could check out my branch and do some testing, that would be nice.
This covers #80