bpo-33967: singledispatch raises TypeError when no positional arguments by corona10 · Pull Request #8184

bpo-33967: singledispatch raises TypeError when no positional arguments by corona10 · Pull Request #8184 · python/cpython

I think this is too much.

I think this is enough:

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

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.

Okay can I update this PR into suggested one?

Choose a reason for hiding this comment

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

Test seems too redundant for now.
TypeError happens always when no positional arguments.
How the generic function defined is not important.

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.

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

methane changed the title ~~bpo-33967: Improve functools.singledispatch exception messages~~ bpo-33967: singledispatch raises TypeError when no positional arguments

Jul 9, 2018

Choose a reason for hiding this comment

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

Error message is not important here. (We won't backport such improvements)

functools.singledispatch now raises TypeError instead of IndexError when no
positional arguments are passed.

Choose a reason for hiding this comment

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

func can be not having the __name__ attribute in general case. This PR can break a code which use singledispatch() with custom callables.

Choose a reason for hiding this comment

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

Also this change creates a new reference to func linked to from wrapper. It may be better to keep a reference just to the name. E.g.

funcname = getattr(func, '__name__', 'singledispatch function')
def wrapper(*args, **kw):
    # use funcname

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

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 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>

		def f(*args):
		pass
		msg = 'f requires at least 1 positional argument'
		with self.assertRaisesRegexp(TypeError, msg):

Choose a reason for hiding this comment

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

assertRaisesRegexp() is deprecated, so this test fails when test_functools is run with -Werror (see #8261).