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

Type hints and changes to class inheritance #76

Open
wants to merge 33 commits into
base: master
Choose a base branch
from

Conversation

bmos
Copy link
Contributor

@bmos bmos commented Feb 4, 2024

This changes from the MixIn pattern to direct inheritance (since it only inherits from one class) and adds type hints to pretty much every arg and return. Since the MixIn was not combining many classes, this should work just as well and it's better supported by the 'magic' of modern IDEs.

It passes the limited pytest implementation, but I'm not so familiar with this module and it's quite likely that this could break things. So I'd advise against merging it without doing at least some basic tests yourself.

I tried to keep most of these commits standalone so it shouldn't be too hard to skip some of them if they're too opinionated, although once I split it into multiple files it probably gets harder to piecemeal it.

@lordjabez
Copy link
Collaborator

really appreciate the contributions @bmos! I inherited this project a few years back and haven't had the time to give it the love it deserves.

Will do some testing when I get time (hopefully in the next few weeks).

@bmos bmos force-pushed the master branch 16 times, most recently from ef9a06d to d4c8c28 Compare February 29, 2024 16:53
@bmos bmos force-pushed the master branch 2 times, most recently from 2e3e60f to df3db34 Compare March 1, 2024 01:59
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.

None yet

2 participants