◐ Shell
reader mode source ↗
Skip to content

bpo-33967: singledispatch raises TypeError when no positional arguments#8184

Merged
methane merged 1 commit into
python:masterfrom
corona10:bpo-33967
Jul 10, 2018
Merged

bpo-33967: singledispatch raises TypeError when no positional arguments#8184
methane merged 1 commit into
python:masterfrom
corona10:bpo-33967

Conversation

@corona10

@corona10 corona10 commented Jul 8, 2018

Copy link
Copy Markdown
Member

@methane

methane commented Jul 9, 2018

Copy link
Copy Markdown
Member

I think this is too much.

I think this is enough:

if not args:
    raise TypeError("singledispatch requires at least 1 positional argument")

@methane methane requested a review from ambv July 9, 2018 09:15
@doerwalter

Copy link
Copy Markdown
Contributor

True, both f(x=3) and g(*args) can be called with one argument, so shouldn't be rejected. I think @methane's solution does the right thing.

@corona10

corona10 commented Jul 9, 2018

Copy link
Copy Markdown
Member Author

Okay can I update this PR into suggested one?

@corona10

corona10 commented Jul 9, 2018

Copy link
Copy Markdown
Member Author

@doerwalter @methane

Updated! Please take a look!

@methane

methane commented Jul 9, 2018

Copy link
Copy Markdown
Member

f"{func.__name__} requires at least 1 positional argument" may be better, if we can assume func.__name__ is available always.

I believe singledispatch requires positional argument by design, intentionally.
But I want @ambv review before merge.

@corona10

corona10 commented Jul 9, 2018

Copy link
Copy Markdown
Member Author

@methane
Thanks for the quick review.
Updated!
One more thing, we need to add tags needs backport to 3.7 / 3.6.

@methane methane changed the title bpo-33967: Improve functools.singledispatch exception messages Jul 9, 2018
@methane methane added type-bug An unexpected behavior, bug, or error needs backport to 3.6 labels Jul 9, 2018
@corona10

corona10 commented Jul 9, 2018

Copy link
Copy Markdown
Member Author

@methane I've updated news

@corona10

Copy link
Copy Markdown
Member Author

I fixed it

@methane methane merged commit 445f1b3 into python:master Jul 10, 2018
@miss-islington

Copy link
Copy Markdown
Contributor

Thanks @corona10 for the PR, and @methane for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7.
🐍🍒⛏🤖

@bedevere-bot

Copy link
Copy Markdown

GH-8220 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 10, 2018
…H-8184)

(cherry picked from commit 445f1b3)

Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>
@bedevere-bot

Copy link
Copy Markdown

GH-8221 is a backport of this pull request to the 3.6 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 10, 2018
…H-8184)

(cherry picked from commit 445f1b3)

Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>
miss-islington added a commit that referenced this pull request Jul 10, 2018
(cherry picked from commit 445f1b3)

Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>
miss-islington added a commit that referenced this pull request Jul 10, 2018
(cherry picked from commit 445f1b3)

Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>
@corona10 corona10 deleted the bpo-33967 branch November 22, 2018 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-bug An unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants