Skip to content

Apply 2to3 next fix.#2134

Merged
takluyver merged 1 commit into
ipython:masterfrom
bfroehle:2to3_next
Jul 15, 2012
Merged

Apply 2to3 next fix.#2134
takluyver merged 1 commit into
ipython:masterfrom
bfroehle:2to3_next

Conversation

@bfroehle

@bfroehle bfroehle commented Jul 13, 2012

Copy link
Copy Markdown
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

takluyver commented Jul 13, 2012

Copy link
Copy Markdown
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

bfroehle
bfroehle reviewed Jul 13, 2012
View reviewed changes
Comment thread IPython/external/pexpect/_pexpect.py Outdated

@bfroehle bfroehle Jul 13, 2012

Copy link
Copy Markdown
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...

@takluyver takluyver Jul 13, 2012

Copy link
Copy Markdown
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.

@bfroehle bfroehle Jul 13, 2012

Copy link
Copy Markdown
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.

@bfroehle
Apply 2to3 next fix.
30e4a14 
Manually set `next = __next__` for Python 2 support.
@takluyver

takluyver commented Jul 14, 2012

Copy link
Copy Markdown
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

takluyver commented Jul 14, 2012

Copy link
Copy Markdown
Member

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

minrk
minrk reviewed Jul 14, 2012
View reviewed changes
Comment thread IPython/zmq/iostream.py

@minrk minrk Jul 14, 2012

Copy link
Copy Markdown
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

minrk commented Jul 14, 2012

Copy link
Copy Markdown
Member

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

minrk commented Jul 14, 2012

Copy link
Copy Markdown
Member

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

minrk commented Jul 14, 2012

Copy link
Copy Markdown
Member

2.6 is happy, +1 to merge.

takluyver added a commit that referenced this pull request Jul 15, 2012
@takluyver
Merge pull request #2134 from bfroehle/2to3_next
87d86c1 
Apply 2to3 `next` fix.
@takluyver takluyver merged commit 87d86c1 into ipython:master Jul 15, 2012
@takluyver

takluyver commented Jul 15, 2012

Copy link
Copy Markdown
Member

Merged - thanks.

@bfroehle bfroehle mentioned this pull request Jul 15, 2012
Merged
@bfroehle bfroehle mentioned this pull request Sep 27, 2012
Closed
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
@takluyver
Merge pull request ipython#2134 from bfroehle/2to3_next
81ee0b6 
Apply 2to3 `next` fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.0

Development

Successfully merging this pull request may close these issues.

3 participants

@bfroehle @takluyver @minrk