-
Notifications
You must be signed in to change notification settings - Fork 11
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
Remove errant __init__.py
that prevents mypy from working
#54
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #54 +/- ##
===========================================
- Coverage 45.19% 45.04% -0.16%
===========================================
Files 42 42
Lines 4290 4318 +28
Branches 876 885 +9
===========================================
+ Hits 1939 1945 +6
- Misses 2173 2193 +20
- Partials 178 180 +2
|
@gpetretto I had a go at fixing some of the typing issues suggested by mypy but I'm not entirely convinced by a lot of the annotations I am adding. Would you be able to take a look? (probably before merging this?) |
Sure. Do you want me to try to apply this change locally and check the typing issues? Or do you already have a list of problematic ones to be verified? |
I'll push the changes I'm happy with to this PR and try to point out the ones that are weird |
I've just pushed these, the remaining lint errors are the weird ones (mostly lots of awkward typing around the db potentially returning |
55a883f
to
ad5a924
Compare
I checked some of the errors related to the The other errors could probably be resolved by adding I am not sure about replacing the values of the Enums with the Enums instance in the CLI. Does it work? Should I test it? |
I've finally got around to fixing the mypy issues in b7b7f81, mostly by just adding checks for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am fine with most of the changes, but some of those to the CLI seem to break Typer. Does it work for you? Otherwise I would revert them and ignore any related linter comments.
Annoyingly this is now back in the state where the tests work locally but not in the CI; I'll try to debug it when I get a chance but for now do not let this PR hold up the other ones that actually add features. |
After your last commit it seems that only a lint issue remains in the tests. Depending on your availability, I suppose these can be fixed and then we can merge this before the other PRs. |
Think that's got it -- I won't be able to take a look at the other PRs again until Friday so feel free to do with this as you please! |
The file at
src/__init__.py
is not necessary and is somehow breaking mypy for the rest of the project. This PR removes that file, then adds a series of typing related fixes.