Skip to content
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

Enforce Abstract #137

Merged
merged 8 commits into from
Apr 6, 2022
Merged

Enforce Abstract #137

merged 8 commits into from
Apr 6, 2022

Conversation

fedem-p
Copy link
Member

@fedem-p fedem-p commented Apr 1, 2022

making abstract classes and interfaces inherit from abc:

  • would prevent to initialize an instance of an abstract class
  • would prevent to initialize a class without having overwritten all the abstract methods

Copy link
Member

@vigji vigji left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me! @vilim? @diegoasua?

@diegoasua
Copy link
Member

diegoasua commented Apr 1, 2022

Love this consistency. Clear and strong 👍 . Just remember to lint it before merging and good to go

@fedem-p
Copy link
Member Author

fedem-p commented Apr 1, 2022

Thanks for the approval @diegoasua @vigji !
Unfortunately it's still in progress, i need to finish changing a couple of files.

Sorry if it wasn't clear from the description

@diegoasua
Copy link
Member

Oh I did not see this was a draft, cool work, let me know when it's finished for a re-review

@fedem-p fedem-p self-assigned this Apr 1, 2022
@fedem-p fedem-p added the Refactor Improve source code readability, reusability or structure label Apr 1, 2022
@coveralls
Copy link

coveralls commented Apr 1, 2022

Pull Request Test Coverage Report for Build 2101667796

  • 88 of 98 (89.8%) changed or added relevant lines in 8 files are covered.
  • 10 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.008%) to 79.965%

Changes Missing Coverage Covered Lines Changed/Added Lines %
sashimi/hardware/cameras/mock.py 9 10 90.0%
sashimi/hardware/light_source/cobolt.py 1 2 50.0%
sashimi/hardware/light_source/mock.py 3 4 75.0%
sashimi/hardware/scanning/init.py 19 21 90.48%
sashimi/hardware/scanning/mock.py 31 36 86.11%
Files with Coverage Reduction New Missed Lines %
sashimi/hardware/light_source/interface.py 1 81.58%
sashimi/hardware/cameras/interface.py 3 80.95%
sashimi/hardware/scanning/init.py 6 74.65%
Totals Coverage Status
Change from base Build 2077058022: 0.008%
Covered Lines: 2311
Relevant Lines: 2890

💛 - Coveralls

@fedem-p fedem-p marked this pull request as ready for review April 1, 2022 14:18
Copy link
Member

@vilim vilim left a comment

Choose a reason for hiding this comment

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

LGTM!

sashimi/hardware/cameras/interface.py Outdated Show resolved Hide resolved
sashimi/hardware/light_source/interface.py Show resolved Hide resolved
shutdown method non abstract
@fedem-p fedem-p merged commit fc59ca5 into master Apr 6, 2022
@fedem-p fedem-p deleted the enforce_abstract branch April 6, 2022 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Refactor Improve source code readability, reusability or structure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants