Skip to content

Replace list.sort() with sorted()#52

Closed
skuicloud wants to merge 1 commit into
testing-cabal:masterfrom
skuicloud:master
Closed

Replace list.sort() with sorted()#52
skuicloud wants to merge 1 commit into
testing-cabal:masterfrom
skuicloud:master

Conversation

@skuicloud

Copy link
Copy Markdown
Contributor

In OpenStack heatclient project, there is error running in py33 env:
https://bugs.launchpad.net/python-heatclient/+bug/1243096

For the Python 2&3 compatability, use sorted() builtin to replace
list.sort().

This change is verified in OpenStack heatclient/novaclient projects.
(tox -e py27 / tox -e py33)

In OpenStack heatclient project, there is error running in py33 env:
https://bugs.launchpad.net/python-heatclient/+bug/1243096

For the Python 2&3 compatability, use sorted() builtin to replace
list.sort().

This change is verified in OpenStack heatclient/novaclient projects.
(tox -e py27 / tox -e py33)
@jml

jml commented Oct 22, 2013

Copy link
Copy Markdown
Member

I don't think the problem is Python 3.3 compatibility. The docs are pretty clear that list.sort works.

Instead of replacing with sorted(), please just pass the key function to sort.

@rbtcollins

Copy link
Copy Markdown
Member

I think you've misinterpreted the problem -

class Foo:
... pass
...
class Bar:
... pass
...
[Foo(), Bar()].sort()
Traceback (most recent call last):
File "", line 1, in
TypeError: unorderable types: Bar() < Foo()

Thats the issue, and sorted has the problem too:

list(sorted([Foo(), Bar()]))
Traceback (most recent call last):
File "", line 1, in
TypeError: unorderable types: Bar() < Foo()

so the thing letting your tests pass is the use of key, which can be used in list.sort() as well.

  • tests = sorted(tests, key=lambda x: x[0])

However, list.sort is very efficient, so please don't change to sorted.

@rbtcollins

Copy link
Copy Markdown
Member

In fact, a further thought occurs to me. The fact the objects are being compared at all indicates you have identical sort keys in that list. As the sort keys are the test ids, this indicates you have a bad test id in your test suite, and even if it runs you'll get bad data out. So this change isn't needed - but please open a bug that we don't give a good error message about your corrupt test suite.

@skuicloud

Copy link
Copy Markdown
Contributor Author

Agree with you, new bug is raised for this:
#56

I will make some further investigation to find the root cause of failed test id.

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.

3 participants