gh-98284: better error message for undefined abstractmethod by kaushikcfd · Pull Request #97971 · python/cpython

kaushikcfd · 2022-10-06T16:39:13Z

Error message before:

TypeError: Can't instantiate abstract class IndexNode with abstract method name

Error message after this patch:

TypeError: Can't instantiate abstract class IndexNode with abstract method 'name'

Given that the style-guide requires method names to be in lowercase, the quotation marks help in differentiating the sentence from the method's name.

Issue: TypeError for undefined abstractmethod does not highlight the undefined method name #98284

bedevere-bot · 2022-10-06T16:39:17Z

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

ghost · 2022-10-06T16:39:39Z

All commit authors signed the Contributor License Agreement.

bedevere-bot · 2022-10-06T18:09:08Z

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

bedevere-bot · 2022-10-06T18:10:06Z

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

JelleZijlstra · 2022-10-07T00:56:50Z

 def method_two(self):
 pass
- msg = r"class C without an implementation for abstract methods method_one, method_two"
+ msg = r"class C without an implementation for abstract methods 'method_one, method_two'"


This is incorrect, we should quote each method individually.

Thanks, done!

bedevere-bot · 2022-10-07T01:09:28Z

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

bedevere-bot · 2022-10-09T14:47:28Z

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

bedevere-bot · 2022-10-09T15:09:58Z

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

bedevere-bot · 2022-10-09T15:13:45Z

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

bedevere-bot · 2022-10-09T15:28:46Z

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

bedevere-bot · 2022-10-09T15:35:52Z

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

bedevere-bot · 2022-10-09T22:35:41Z

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

JelleZijlstra · 2022-10-09T22:55:00Z

You do need a NEWS entry as the good knight Bedevere keeps telling you.

Also, avoid force-pushing; it makes review harder. We'll squash-merge anywhere when we merge a PR.

kaushikcfd · 2022-10-09T23:09:42Z

You do need a NEWS entry as the good knight Bedevere keeps telling you.

Done!

Also, avoid force-pushing;

Noted, thanks!

JelleZijlstra · 2022-10-14T03:09:17Z

@@ -0,0 +1,3 @@
+Improved :class:`TypeError` message for undefined abstract methods of a


The NEWS entry must refer to an issue, not a PR. You'll have to create an issue and rename the file to have your issue number in the file name.

Thanks, see 4073b1f.

kaushikcfd · 2022-10-15T16:12:55Z

The CI failure is (most likely) unrelated:

PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'D:\\a\\cpython\\cpython\\build\\test_python_2628�\\test_python_worker_1152�'

sobolevn · 2022-10-15T17:17:02Z

Retriggering CI, it is indeed unrelated.

kaushikcfd · 2022-10-15T18:13:18Z

Retriggering CI, it is indeed unrelated.

Thanks, yep that was it.

kaushikcfd · 2022-11-04T17:34:10Z

Soft ping.

JelleZijlstra · 2022-11-05T15:42:04Z

 method_count > 1 ? "s" : "",
 joined);
 Py_DECREF(joined);
+ Py_DECREF(comma_w_quotes_sep);


You also need to DECREF this in the two return NULL branches above.
Come to think of it, the second of those already leaks joined. It needs a Py_DECREF(joined).

Aah, yep! Fixed in 6ae052e.

kaushikcfd requested a review from markshannon as a code owner October 6, 2022 16:39

bedevere-bot added the awaiting review label Oct 6, 2022

kaushikcfd force-pushed the abc_better_err_msg branch from 726f869 to 34333a3Compare October 6, 2022 18:09

kaushikcfd force-pushed the abc_better_err_msg branch from 34333a3 to 17e8982Compare October 6, 2022 18:10

kaushikcfd requested a review from ericvsmith as a code owner October 6, 2022 18:10

JelleZijlstra reviewed Oct 7, 2022
View reviewed changes

kaushikcfd force-pushed the abc_better_err_msg branch from 17e8982 to 3edd66eCompare October 7, 2022 01:09

kaushikcfd force-pushed the abc_better_err_msg branch from 3edd66e to c351ac2Compare October 9, 2022 14:47

kaushikcfd force-pushed the abc_better_err_msg branch from c351ac2 to f5a9680Compare October 9, 2022 15:09

kaushikcfd force-pushed the abc_better_err_msg branch from f5a9680 to 53e87c5Compare October 9, 2022 15:13

kaushikcfd force-pushed the abc_better_err_msg branch from 53e87c5 to b2b2d95Compare October 9, 2022 15:28

kaushikcfd force-pushed the abc_better_err_msg branch from b2b2d95 to 820cc21Compare October 9, 2022 15:35

kaushikcfd requested review from JelleZijlstra and removed request for ericvsmith and markshannon October 9, 2022 16:10

[abc] better error message for undefined abstractmethod
c268395

kaushikcfd force-pushed the abc_better_err_msg branch from 820cc21 to c268395Compare October 9, 2022 22:35

adds a NEWS entry
3603740

JelleZijlstra reviewed Oct 14, 2022
View reviewed changes

kaushikcfd mentioned this pull request Oct 15, 2022
TypeError for undefined abstractmethod does not highlight the undefined method name #98284
Closed

kaushikcfd changed the title ~~[abc] better error message for undefined abstractmethod~~gh-98284: better error message for undefined abstractmethodOct 15, 2022

kaushikcfd added 2 commits October 15, 2022 10:44

corrects the NEWS entry
4073b1f

Merge remote-tracking branch 'upstream/main' into abc_better_err_msg
6a762e4

sobolevn closed this Oct 15, 2022

sobolevn reopened this Oct 15, 2022

JelleZijlstra reviewed Nov 5, 2022
View reviewed changes

Avoid leaks by calling Py_DECREF in some code paths
6ae052e

JelleZijlstra approved these changes Nov 5, 2022
View reviewed changes

bedevere-bot added awaiting merge and removed awaiting review labels Nov 5, 2022

JelleZijlstra self-assigned this Nov 5, 2022

JelleZijlstra merged commit 67ade40 into python:mainNov 5, 2022

bedevere-bot removed the awaiting merge label Nov 5, 2022

		@@ -0,0 +1,3 @@
		Improved :class:`TypeError` message for undefined abstract methods of a

Uh oh!

Uh oh!

Conversation

kaushikcfd commented Oct 6, 2022• edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-bot commented Oct 6, 2022

Uh oh!

ghost commented Oct 6, 2022• edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-bot commented Oct 6, 2022

Uh oh!

bedevere-bot commented Oct 6, 2022

Uh oh!

JelleZijlstraOct 7, 2022

Choose a reason for hiding this comment

Uh oh!

kaushikcfdOct 9, 2022

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Oct 7, 2022

Uh oh!

bedevere-bot commented Oct 9, 2022

Uh oh!

bedevere-bot commented Oct 9, 2022

Uh oh!

bedevere-bot commented Oct 9, 2022

Uh oh!

bedevere-bot commented Oct 9, 2022

Uh oh!

bedevere-bot commented Oct 9, 2022

Uh oh!

bedevere-bot commented Oct 9, 2022

Uh oh!

JelleZijlstra commented Oct 9, 2022

Uh oh!

kaushikcfd commented Oct 9, 2022

Uh oh!

JelleZijlstraOct 14, 2022

Choose a reason for hiding this comment

Uh oh!

kaushikcfdOct 15, 2022

Choose a reason for hiding this comment

Uh oh!

kaushikcfd commented Oct 15, 2022

Uh oh!

sobolevn commented Oct 15, 2022

Uh oh!

kaushikcfd commented Oct 15, 2022

Uh oh!

kaushikcfd commented Nov 4, 2022

Uh oh!

JelleZijlstraNov 5, 2022

Choose a reason for hiding this comment

Uh oh!

kaushikcfdNov 5, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kaushikcfd commented Oct 6, 2022•
edited by bedevere-bot
Loading

ghost commented Oct 6, 2022•
edited by ghost
Loading