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

feat: add mypy to the scripts/ module #671

Merged
merged 4 commits into from
Oct 30, 2023
Merged

feat: add mypy to the scripts/ module #671

merged 4 commits into from
Oct 30, 2023

Conversation

joyceyan
Copy link
Contributor

Reason for Change

Changes

  • adds mypy config to pyproject.toml
  • adds a make command, make mypy, which runs mypy --config-file pyproject.toml
  • adds the run_mypy_and_annotate.py script to the root directory of the project, which runs the following command: mypy --config-file pyproject.toml and annotates offending lines with # type: ignore
  • adds a pre-commit hook to run mypy

Testing

  • ran the script, and verified make mypy can run with no issues

Notes for Reviewer

@codecov
Copy link

codecov bot commented Oct 19, 2023

Codecov Report

Merging #671 (80fa2d3) into main (f7f1148) will not change coverage.
The diff coverage is 87.50%.

@@           Coverage Diff           @@
##             main     #671   +/-   ##
=======================================
  Coverage   84.73%   84.73%           
=======================================
  Files          19       19           
  Lines        1815     1815           
=======================================
  Hits         1538     1538           
  Misses        277      277           
Flag Coverage Δ
unittests 84.73% <87.50%> (ø)

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

Files Coverage Δ
...lxgene_schema_cli/cellxgene_schema/write_labels.py 95.26% <ø> (ø)
scripts/logger.py 100.00% <100.00%> (ø)
...migration_assistant/tests/test_convert_template.py 100.00% <100.00%> (ø)
...bump_dry_run_genes/tests/test_gene_bump_dry_run.py 100.00% <100.00%> (ø)
...a_bump_dry_run_ontologies/ontology_bump_dry_run.py 95.09% <100.00%> (ø)
...run_ontologies/tests/test_ontology_bump_dry_run.py 100.00% <100.00%> (ø)
cellxgene_schema_cli/cellxgene_schema/ontology.py 93.90% <88.88%> (ø)
scripts/common/thirdparty/discovery_api.py 53.33% <80.00%> (ø)
scripts/migration_assistant/generate_script.py 76.31% <60.00%> (ø)
...pts/schema_bump_dry_run_genes/gene_bump_dry_run.py 60.62% <64.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

run_mypy_and_annotate.py Outdated Show resolved Hide resolved
@joyceyan joyceyan requested a review from atolopko-czi October 20, 2023 18:41
Copy link
Contributor

@atolopko-czi atolopko-czi left a comment

Choose a reason for hiding this comment

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

Some comments for your review before merging, but overall looks reasonable!

Also, @Bento007 might be another worthwhile reviewer on this.

pyproject.toml Show resolved Hide resolved
run_mypy_and_annotate.py Outdated Show resolved Hide resolved
run_mypy_and_annotate.py Outdated Show resolved Hide resolved
.github/workflows/push_tests.yml Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
run_mypy_and_annotate.py Outdated Show resolved Hide resolved
Copy link
Contributor

@atolopko-czi atolopko-czi left a comment

Choose a reason for hiding this comment

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

LGTM! Just one comment suggestion.

.github/workflows/push_tests.yml Show resolved Hide resolved
@joyceyan joyceyan merged commit 3174810 into main Oct 30, 2023
8 checks passed
@joyceyan joyceyan deleted the joyce/mypy-scripts branch October 30, 2023 16:25
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.

2 participants