Skip to content

Conversation

@tobsecret
Copy link
Contributor

@tobsecret tobsecret commented Apr 24, 2025

Canonically @abc.abstractmethod is used to designate methods that are mandatory for subclasses to implement - otherwise those classes fail at instantiation. This behavior is only active if the class inherits from abc.ABC or analogons.
However, @abc.abstractmethod can also be used to simply add the __isabstractmethod__ property to a class or instance method which is how it was used in AbstractApplication.

I have reconfigured the AbstractApplication to inherit from abc.ABC and have replaced the logic that @abc.abstractmethod was used for previously with is_implemented.

There is some discussion to be had about the exact implementation of is_implemented. Currently it is hardcoded to check whether a method of specifically AbstractApplication was overridden, as opposed to the base class of the instance self.__class__.__bases__[0].

  • 🐛   Bug fix
  • 🚀   New feature
  • 🔧   Code refactor
  • ⚠️   Breaking change that would cause existing functionality to change
  • 📘   I have updated the documentation accordingly
  • ✅   I have added tests to cover my changes

@codecov
Copy link

codecov bot commented May 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.55%. Comparing base (4b8dadc) to head (9daba61).

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #74   +/-   ##
=======================================
  Coverage   95.54%   95.55%           
=======================================
  Files          19       19           
  Lines        2381     2386    +5     
=======================================
+ Hits         2275     2280    +5     
  Misses        106      106           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tobsecret tobsecret changed the title Reconfigured AbstractApplication to use abc.ABC Reconfigured AbstractApplication to inherit from abc.ABC May 6, 2025
@juanesarango juanesarango self-requested a review May 6, 2025 21:10
Copy link
Contributor

@juanesarango juanesarango left a comment

Choose a reason for hiding this comment

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

Great! Thanks for the contribution fixing this @tobsecret!

Allow me to run a couple of tests on our apps using this branch, before merging it.

@tobsecret
Copy link
Contributor Author

@juanesarango sure thing! Thanks for the pairing session today!
As the maintainer you should be able to just press the merge button when you're done running your extra tests - I'll leave it up to you.

@miraep8
Copy link

miraep8 commented May 8, 2025

Looks good to me 👍 Thanks for working on this team!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants