Skip to content

Conversation

@DimitriPapadopoulos
Copy link
Contributor

@DimitriPapadopoulos DimitriPapadopoulos commented Sep 25, 2024

[Description of PR]

TODO:

  • Add unit tests and/or doctests in docstrings
  • Add docstrings and API docs for any new/modified user-facing classes and functions
  • New/modified features documented in docs/tutorial.rst
  • Changes documented in docs/release.rst
  • GitHub Actions have all passed
  • Test coverage is 100% (Codecov passes)

@DimitriPapadopoulos DimitriPapadopoulos marked this pull request as ready for review September 25, 2024 08:43
@TomAugspurger
Copy link
Contributor

Thanks. Out of curiosity, did you hit a problem with this? https://docs.python.org/3/library/abc.html#abc.abstractmethod notes that the abstractmethod decorator will prevent you from even creating an instance of the class, so this code should be unreachable.

@DimitriPapadopoulos
Copy link
Contributor Author

No real problem. I am merely trying to silence linter rules (deepsource). However, some of these rules don't really make sense. Rule PTC-W0053 is described as follows:

Abstract methods should raise NotImplementeError 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.

Refer to the docs to read more about NotImplementedError.

The link to the Python docs reads:

exception NotImplementedError
This exception is derived from 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.

I guess the implementers of the linter just read and implemented that paragraph naively, without realising that @abstractmethod is equivalent. Perhaps we should just disregard the linker and close this issue.

@jhamman
Copy link
Member

jhamman commented Sep 26, 2024

Perhaps we should just disregard the linker and close this issue.

+1

Thanks for all your help on the code quality front @DimitriPapadopoulos!

@jhamman jhamman added this to the 3.0.0 milestone Sep 26, 2024
@DimitriPapadopoulos DimitriPapadopoulos deleted the PTC-W0053 branch September 26, 2024 05:49
@jhamman jhamman added the V3 label Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants