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

Apply 2to3 next fix. #2134

Merged
merged 1 commit into from Jul 15, 2012
Merged

Apply 2to3 next fix. #2134

merged 1 commit into from Jul 15, 2012

Conversation

bfroehle
Copy link
Contributor

Continuing on the Python 2/3 compatibility (which I'll refer to as '2and3') from #2095 and #2100, I propose to include the changes from running 2to3 -f next:

Converts the use of iterator’s next() methods to the next() function. It also renames next() methods to __next__().

One small issue is the behavior of next(x) on user defined classes:

Consider test_next.py

class X(object):
    def __init__(self, iterable):
        self.iterator = iter(iterable)
    def __iter__(self):
        return self
    def __next__(self):
        print("__next__ called")
        return next(self.iterator)
    def next(self):
        print("next called")
        return next(self.iterator)

x = X([1,2,3])
next(x)

And run it in Python 2.7 and 3.2:

$ python2.7 next_test.py
next called
$ python3.2 next_test.py
__next__ called

To work around this difference, I've just set next = __next__ manually after applying the 2to3 fix. This also maintains compatibility with any third party code which calls x.next().

@takluyver
Copy link
Member

Test results for commit 360c5cd merged into master
Platform: linux2

  • python2.7: OK (libraries not available: oct2py rpy2)
  • python3.1: OK (libraries not available: cython matplotlib numpy oct2py pymongo qt rpy2 wx wx.aui zmq)
  • python3.2: OK (libraries not available: cython oct2py pymongo rpy2 wx wx.aui)

Not available for testing: python2.6

@@ -940,6 +940,8 @@ def next (self): # File-like object.
raise StopIteration
return result

next = __next__ # File-like object. (Python 2).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alternatively we could treat this as:

    def __next(self)__:
        # ...

    if not py3compat.PY3:
        next = __next__

This would make it more obvious what needs to be ripped out if Python 2 support was ever dropped...

Copy link
Member

Choose a reason for hiding this comment

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

I think I'd side with doing that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. I've made the changes and pushed a new commit.

Manually set `next = __next__` for Python 2 support.
@takluyver
Copy link
Member

Test results for commit 30e4a14 merged into master
Platform: linux2

  • python2.7: OK (libraries not available: oct2py rpy2)
  • python3.1: OK (libraries not available: cython matplotlib numpy oct2py pymongo qt rpy2 wx wx.aui zmq)
  • python3.2: OK (libraries not available: cython oct2py pymongo rpy2 wx wx.aui)

Not available for testing: python2.6

@takluyver
Copy link
Member

This looks OK to me. If no-one objects in about a day, we can merge it.

raise IOError('Read not supported on a write only stream.')

if not py3compat.PY3:
next = __next__

Copy link
Member

Choose a reason for hiding this comment

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

Is there a good reason that this method is defined in a conditional, and not just next = __next__? I know the method is not required in py3, but it seems cleaner to just define it.

@minrk
Copy link
Member

minrk commented Jul 14, 2012

Ah, now I just saw the conversation above. I don't really agree with the decision, but I don't feel strongly, so I'll go with you guys. Also, Python 2.6 testing is the most important here before merge.

@minrk
Copy link
Member

minrk commented Jul 14, 2012

Test results for commit 30e4a14 merged into master
Platform: darwin

  • python2.6: OK (libraries not available: cython matplotlib oct2py pygments pymongo qt rpy2 tornado wx wx.aui)
  • python2.7: OK (libraries not available: oct2py wx wx.aui)
  • python3.2: OK (libraries not available: cython matplotlib oct2py pymongo qt rpy2 wx wx.aui)

Not available for testing: python3.1

@minrk
Copy link
Member

minrk commented Jul 14, 2012

2.6 is happy, +1 to merge.

takluyver added a commit that referenced this pull request Jul 15, 2012
@takluyver takluyver merged commit 87d86c1 into ipython:master Jul 15, 2012
@takluyver
Copy link
Member

Merged - thanks.

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
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.

None yet

3 participants