Skip to content

Add mypy to the pipeline#7383

Merged
auvipy merged 8 commits intocelery:masterfrom
Kludex:typing/annotations-v2
Apr 2, 2022
Merged

Add mypy to the pipeline#7383
auvipy merged 8 commits intocelery:masterfrom
Kludex:typing/annotations-v2

Conversation

@Kludex
Copy link
Copy Markdown
Contributor

@Kludex Kludex commented Mar 29, 2022 •
edited
Loading

Thanks @atombrella for the initial work!

This PR adds mypy to tox and to the pipeline. A couple of differences between this PR and the previous one:

  • Apply mypy per file. Meaning that we can do progressive work just adding a new file to the pyproject.toml.
  • Use pyproject.toml instead of mypy.ini.
  • disallow_untyped_defs is True now.

This PR is intended to be minimal, so we can start working per files basis. I've reused the previous branch, so there are also some small type annotation fixes, which are not required for this PR.

atombrella and others added 3 commits February 28, 2022 13:34
@atombrella
Add typing to Celery
edd9dc4 
This is a simple bootstrap of the process, adding some types to a
few selected functions, based on comment annotations. MyPy is chosen as the
default static analyzer for the types.
@Kludex
Add mypy to the pipeline
436ccfe
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
b720d6b 
for more information, see https://pre-commit.ci
@Kludex Kludex changed the title Typing/annotations v2 Add mypy to the pipeline Mar 29, 2022
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 29, 2022 •
edited
Loading

Codecov Report

Merging #7383 (7617ec3) into master (8423c67) will decrease coverage by 0.02%.
The diff coverage is 85.71%.

@@            Coverage Diff             @@
##           master    #7383      +/-   ##
==========================================
- Coverage   89.33%   89.30%   -0.03%     
==========================================
  Files         138      138              
  Lines       16781    16762      -19     
  Branches     2451     2451              
==========================================
- Hits        14991    14970      -21     
- Misses       1559     1560       +1     
- Partials      231      232       +1
Flag Coverage Δ
unittests 89.30% <85.71%> (-0.03%) ⬇️

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

Impacted Files Coverage Δ
celery/__main__.py 0.00% <0.00%> (ø)
celery/events/state.py 97.22% <100.00%> (-0.03%) ⬇️
celery/utils/collections.py 85.84% <100.00%> (+0.15%) ⬆️
celery/backends/asynchronous.py 67.53% <0.00%> (-2.17%) ⬇️
celery/bin/worker.py 45.31% <0.00%> (-1.47%) ⬇️
celery/result.py 96.28% <0.00%> (-0.07%) ⬇️
celery/beat.py 91.54% <0.00%> (-0.05%) ⬇️
celery/app/defaults.py 97.33% <0.00%> (ø)
celery/contrib/abortable.py 100.00% <0.00%> (ø)
... and 4 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 095cd78...7617ec3. Read the comment docs.

@Kludex Kludex mentioned this pull request Mar 29, 2022
Closed
@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Mar 29, 2022

This pull request fixes 3 alerts when merging bdee5e0 into 24f22a5 - view on LGTM.com

fixed alerts:

  • 1 for Non-exception in 'except' clause
  • 1 for Unused import
  • 1 for Module is imported with 'import' and 'import from'

@povilasb
Copy link
Copy Markdown

povilasb commented Mar 29, 2022

Nice, type hints are much appreciated when navigating the code base.
Thank you!

@Kludex
Copy link
Copy Markdown
Contributor Author

Kludex commented Mar 29, 2022 •
edited
Loading

Yep. As soon as this gets in, I'll organize a similar issue as I did on uvicorn: Kludex/uvicorn#998

Then we can start working on it, in an incremental way :)

@auvipy auvipy self-requested a review April 2, 2022 05:48
auvipy
auvipy reviewed Apr 2, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

good job

@Kludex Kludex requested a review from auvipy April 2, 2022 06:33
@Kludex
Copy link
Copy Markdown
Contributor Author

Kludex commented Apr 2, 2022

All the annotations besides __main__.py are not needed for this PR. If you want, I can remove the other ones.

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Apr 2, 2022

This pull request introduces 2 alerts and fixes 1 when merging e2eda10 into 24f22a5 - view on LGTM.com

new alerts:

  • 1 for Unused import
  • 1 for Module is imported with 'import' and 'import from'

fixed alerts:

  • 1 for Unused import

Kludex
Kludex commented Apr 2, 2022
View reviewed changes
@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Apr 2, 2022

This pull request fixes 1 alert when merging 7617ec3 into 24f22a5 - view on LGTM.com

fixed alerts:

  • 1 for Non-exception in 'except' clause

@Kludex
Copy link
Copy Markdown
Contributor Author

Kludex commented Apr 2, 2022

I've just found out about this: Kludex/uvicorn#1437 (comment)

Might be useful for celery as well.

@auvipy auvipy merged commit fbda008 into celery:master Apr 2, 2022
@auvipy auvipy added the CI label Apr 2, 2022
@Kludex
Copy link
Copy Markdown
Contributor Author

Kludex commented Apr 2, 2022

Thanks @auvipy ! :)

I'm going to create an issue so we can organize the work towards a fully annotated celery! 🙌

@auvipy
Copy link
Copy Markdown
Member

auvipy commented Apr 2, 2022

we can just modify this #7258

@Kludex Kludex deleted the typing/annotations-v2 branch April 2, 2022 13:40
@Kludex Kludex mentioned this pull request Apr 2, 2022
Open
@Kludex
Copy link
Copy Markdown
Contributor Author

Kludex commented Apr 2, 2022

@auvipy I didn't modify it because I'd like to have rights to edit. See the created issue: #7394

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

Reviewers

@auvipy auvipy auvipy approved these changes

Assignees

No one assigned

Labels

CI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Kludex @povilasb @auvipy @atombrella