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

add fixer for sorted() and list.sort #200

Closed
wants to merge 4 commits into from

Conversation

squirrel532
Copy link
Contributor

In python3, sorted() and list.sort() only accept keyword-only
argument and drop support of the cmp flag. This fixer helps you
overcome all these trap by wrapping functools.cmp_to_key to original
cmp argument and transforming it to key argument. See
https://docs.python.org/3.8/library/stdtypes.html#list.sort

@squirrel532
Copy link
Contributor Author

This fixer shoud bu optional because .sort method may not be list.sort.

In python3, `sorted()` and `list.sort()` only accept keyword-only
argument and drop support of the `cmp` flag.  This fixer helps you
overcome all these trap by wrapping `functools.cmp_to_key` to original
`cmp` argument and transforming it to `key` argument. See
https://docs.python.org/3.8/library/stdtypes.html#list.sort
@takluyver
Copy link
Contributor

I'd say it's OK for it to it to be on by default, by analogy with the dict fixers that assume e.g x.items() is calling a dict method.

Either way, it should have some tests. Tests for python-modernize are pretty simple: give some input code and the expected output. E.g. here for the map fixer

@squirrel532
Copy link
Contributor Author

@takluyver Could you help me put some description and warning into document ? I'm not sure how to edit it correctly.

@graingert
Copy link
Member

@azdkj532 I think this fixer should be in fissix as it doesn't involve six

@squirrel532
Copy link
Contributor Author

I'll file a PR to fissix.

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