Repository navigation
gh-158911 Docs: Correct and clarify NotImplementedError usage guidelines #158912
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -338,17 +338,27 @@ The following exceptions are the exceptions that are usually raised. | |
|
|
||
| .. exception:: NotImplementedError | ||
|
|
||
| This exception is derived from :exc:`RuntimeError`. In user defined base | ||
| classes, abstract methods should raise this exception when they require | ||
| derived classes to override the method, or while the class is being | ||
| developed to indicate that the real implementation still needs to be added. | ||
| This exception is derived from :exc:`RuntimeError`. In user-defined base | ||
| classes, any **non**-abstract method should raise this exception when derived | ||
| classes are required to override the method, indicating that the real | ||
| implementation still needs to be added. | ||
|
|
||
| .. note:: | ||
|
|
||
| It should not be used to indicate that an operator or method is not | ||
| meant to be supported at all -- in that case either leave the operator / | ||
| method undefined or, if a subclass, set it to :data:`None`. | ||
|
|
||
| .. caution:: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't want this paragraph. This leaks implementation details and relates to a concept that is not about exceptions specifically. |
||
|
|
||
| Methods decorated with :func:`abc.abstractmethod` designate a member function | ||
| as abstract, which prompts the ABC metaclass enforcement mechanism to verify | ||
| that a concrete implementation resides within the instantiated subclass. | ||
| Consequently, the Python interpreter invokes the overridden child class implementation | ||
| directly; the original base class method body remains uncalled during regular | ||
| polymorphic execution, rendering the inclusion of a :exc:`NotImplementedError` | ||
| entirely superfluous and redundant. | ||
|
Comment on lines
+354
to
+360
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Calling a base class method via |
||
|
|
||
| .. caution:: | ||
|
|
||
| :exc:`!NotImplementedError` and :data:`!NotImplemented` are not | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not necessarily true, see
cpython/Lib/_collections_abc.py
Lines 450 to 452 in fb313a3
This makes the intent clearer instead of having a
passstatement or a...statement and allows one to remove the decorator if necessary.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this is wrong. An abstract method never should contain code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Python you must have one code. A
passstatement remains something. So sorry but I'm not accepting this change. Clarity is better than purity in the language and NotImplementedError predates ABCs. ABCs also add overhead at runtime while raising NotImplementedError directly (without any abc.abstractmethod decorator) is the only way to convene the intent of an abstract method.The page about exceptions is not about ABC only. It's for anyone wanting to define an abstract method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A pass statement does not count as code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please review this correctly, your arguments are not logically, who pays you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does, from an interpreter PoV:
Please keep it civil. This would count as a breach of CoC. And I already explained the rationale on the issue and here: ABCs should not be considered by NotImplementedError. They are an alternative where you are allowed to write regular code as well (in case you want to move OUT of ABCs in the future).