Hi. I reported some false positives a while ago, which said that abstract methods should raise
NotImplementedError, even though the class can’t even be instantiated when it inherits from
I was contacted by mail about this:
I noticed you reported a false positive for PTC-W0053 - Abstract method does not raise NotImplementedError.
Thanks for providing a very detailed comment.
The issue was raised for the following snippet in the file telegram/ext/filters.py:
@abstractmethod def __call__(self, update: Update) -> Optional[Union[bool, Dict]]: ...
This issue is raised when an abstract method is left not implemented using
I agree with your comment that a class with an abstract method cannot be initialized.
NotImplementedErrorimproves the code readability. It gives a clear indication and
makes it explicit that the method needs to be implemented in the subclass. It is a sort of
Let me know what are your thoughts on this use case.
Language Engineering, DeepSource
thanks for reaching out.
However, I tend to disagree. IMHO the
@abstractmethod(and even the
..., if you will), are clear indications that the method has to be implemented. Adding
raise NotImplementedErrorcan on the contrary give the impression that the method actually does something.
Moreover, for abstract classes that are part of a public API, the user usually won’t see the code anyway, while any decent IDE will complain (before run-time) if you try to subclass without implementing all the abtsract methods.
I’d agree that an error should not be raised, if the body of an abstract method contains only
raise NotImplementedError, but it shouldn’t me mandatory.
Unfortunately I never heard back on this. Is there any update on the topic? The issue still persists and is one of only two things left before I can actually integrate DS into my project …