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

style: Fixed mypy and moved tool configs to pyproject.toml #966

Merged
merged 16 commits into from
Jul 1, 2022
Merged

Conversation

frgfm
Copy link
Collaborator

@frgfm frgfm commented Jun 28, 2022

As part of #957, this PR introduces the following modifications:

  • moves all tool configs to pyproject.toml (flake8 doesn't support it yet)
  • fixed mypy typing & isort in the entire project

Any feedback is welcome!

@frgfm frgfm added type: enhancement Improvement topic: build Related to dependencies and build type: code quality Related to code quality labels Jun 28, 2022
@frgfm frgfm added this to the 0.6.0 milestone Jun 28, 2022
@frgfm frgfm self-assigned this Jun 28, 2022
@codecov
Copy link

codecov bot commented Jun 28, 2022

Codecov Report

Merging #966 (bacb7f2) into main (fb47752) will increase coverage by 0.00%.
The diff coverage is 95.23%.

@@           Coverage Diff           @@
##             main     #966   +/-   ##
=======================================
  Coverage   94.89%   94.89%           
=======================================
  Files         134      134           
  Lines        5540     5542    +2     
=======================================
+ Hits         5257     5259    +2     
  Misses        283      283           
Flag Coverage Δ
unittests 94.89% <95.23%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
doctr/datasets/cord.py 97.72% <ø> (ø)
doctr/models/artefacts/face.py 95.23% <ø> (ø)
doctr/models/detection/predictor/pytorch.py 94.73% <ø> (ø)
doctr/models/detection/predictor/tensorflow.py 94.44% <ø> (ø)
doctr/models/utils/tensorflow.py 100.00% <ø> (ø)
doctr/utils/data.py 88.67% <0.00%> (ø)
doctr/utils/visualization.py 89.65% <33.33%> (ø)
doctr/models/detection/linknet/base.py 88.69% <85.71%> (ø)
...dels/detection/differentiable_binarization/base.py 93.52% <92.30%> (ø)
doctr/datasets/datasets/base.py 98.00% <100.00%> (ø)
... and 38 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fb47752...bacb7f2. Read the comment docs.

@felixdittrich92
Copy link
Contributor

@frgfm thanks a lot that's really helpful :)

some mypy issues left

@felixdittrich92
Copy link
Contributor

@frgfm I have canceled the jobs (runs since ~2hrs to find matching packages for tf):

https://github.com/mindee/doctr/runs/7091359034?check_suite_focus=true #dependency war ⚠️

@felixdittrich92
Copy link
Contributor

felixdittrich92 commented Jun 28, 2022

@frgfm again :/
I think it would be helpful to upgrade tf first 😅 (we can exclude TF mobilenet models onnx export tests ftm to pass the tensorflow tests)

@frgfm
Copy link
Collaborator Author

frgfm commented Jun 28, 2022

All good now 👍

Copy link
Contributor

@felixdittrich92 felixdittrich92 left a comment

Choose a reason for hiding this comment

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

Thanks a lot really nice 👍
Only one question what about flake (looks like it was not triggered) ? :)

@frgfm
Copy link
Collaborator Author

frgfm commented Jun 28, 2022

Thanks a lot really nice +1 Only one question what about flake (looks like it was not triggered) ? :)

It has been, but I changed the python runtime and renamed the job, so we have to update the "required" jobs :)

@felixdittrich92
Copy link
Contributor

Thanks a lot really nice +1 Only one question what about flake (looks like it was not triggered) ? :)

It has been, but I changed the python runtime and renamed the job, so we have to update the "required" jobs :)

Yeah i see and was currently on track to check this 😅

@felixdittrich92
Copy link
Contributor

@frgfm can we finish this ? :)

@charlesmindee charlesmindee merged commit 82d6e7e into main Jul 1, 2022
@charlesmindee charlesmindee deleted the style branch July 1, 2022 14:15
@felixdittrich92 felixdittrich92 mentioned this pull request Sep 26, 2022
85 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: build Related to dependencies and build type: code quality Related to code quality type: enhancement Improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants