opentelemetry-python icon indicating copy to clipboard operation
opentelemetry-python copied to clipboard

Tox doesn't run mypy on the sdk code

Open aabmass opened this issue 5 years ago • 4 comments
trafficstars

Currently, tox.ini only has mypy running on the api, so the sdk is not covered in CI: https://github.com/open-telemetry/opentelemetry-python/blob/2ad9f497c56117c26fd49ec63d996ed85df36e0f/tox.ini#L254-L260

See the commands in an example run:

$ tox -e mypy
mypy installed: mypy==0.770,mypy-extensions==0.4.3,typed-ast==1.4.1,typing-extensions==3.7.4.2
mypy run-test-pre: PYTHONHASHSEED='1981813759'
mypy run-test: commands[0] | mypy --namespace-packages opentelemetry-api/src/opentelemetry/
Success: no issues found in 21 source files
mypy run-test: commands[1] | mypy --namespace-packages --config-file=mypy-relaxed.ini opentelemetry-api/tests/
Success: no issues found in 20 source files
__________________________________________________________________ summary ___________________________________________________________________
  mypy: commands succeeded
  congratulations :)

aabmass avatar Jun 03 '20 20:06 aabmass

This issue was marked stale due to lack of activity. It will be closed in 30 days.

github-actions[bot] avatar Apr 09 '21 03:04 github-actions[bot]

it'd be nice if we upgraded mypy to > 0.930 so that it can install typed_ast > 1.5.0 - so that this project can work natively on M1. As it stands now it fails to run because [email protected] doesn't compile on M1.

flyinprogrammer avatar Feb 17 '22 03:02 flyinprogrammer

it'd be nice if we upgraded mypy to > 0.930 so that it can install typed_ast > 1.5.0 - so that this project can work natively on M1. As it stands now it fails to run because [email protected] doesn't compile on M1.

Thanks for the suggestion @flyinprogrammer, I've created a separate issue to track that, would you be interested in submitting a PR for it?

codeboten avatar Feb 17 '22 17:02 codeboten

i want to be the hero you're looking for, i think it's gonna take me a minute though 🥲

flyinprogrammer avatar Feb 17 '22 23:02 flyinprogrammer

Does it run now? As it seems, we have upgraded version of mypy to >0.930 Can we close this?

rajat315315 avatar Jul 19 '23 11:07 rajat315315

We would need first to run mypy on the SDK files too:

diff --git a/tox.ini b/tox.ini
index ce3733729..1b712b030 100644
--- a/tox.ini
+++ b/tox.ini
@@ -96,7 +96,7 @@ setenv =
   ; i.e: CONTRIB_REPO_SHA=dde62cebffe519c35875af6d06fae053b3be65ec tox -e <env to test>
   CONTRIB_REPO_SHA={env:CONTRIB_REPO_SHA:"main"}
   CONTRIB_REPO="git+https://github.com/open-telemetry/opentelemetry-python-contrib.git@{env:CONTRIB_REPO_SHA}"
-  mypy: MYPYPATH={toxinidir}/opentelemetry-api/src/:{toxinidir}/tests/opentelemetry-test-utils/src/
+  mypy: MYPYPATH={toxinidir}/opentelemetry-api/src/:{toxinidir}/opentelemetry-sdk/src/:{toxinidir}/tests/opentelemetry-test-utils/src/
 
 changedir =
   api: opentelemetry-api/tests
@@ -195,7 +195,7 @@ commands =
   opentelemetry: pytest {posargs}
   coverage: {toxinidir}/scripts/coverage.sh
 
-  mypy: mypy --install-types --non-interactive --namespace-packages --explicit-package-bases opentelemetry-api/src/opentelemetry/
+  mypy: mypy --install-types --non-interactive --namespace-packages --explicit-package-bases opentelemetry-api/src/opentelemetry/ opentelemetry-sdk/src/opentelemetry/sdk/
 
 ; For test code, we don't want to enforce the full mypy strictness
   mypy: mypy --install-types --non-interactive --namespace-packages --config-file=mypy-relaxed.ini opentelemetry-api/tests/

I tried running tox -e mypy with these changes and noticed many failures.

ocelotl avatar Aug 07 '23 09:08 ocelotl