opentelemetry-python
opentelemetry-python copied to clipboard
Add type hints to opentelemetry-sdk and run as a part of tests
Much of opentelemetry-sdk
code doesn't have type hints. It would be great to add mypy type hints and run the tests similar to opentelemetry-api
.
Related to https://github.com/open-telemetry/opentelemetry-python/issues/773
I've looked into this a few times and there were really annoying issues with namespace packages and finding the code. Mypy 0.8xx (#1575) has made huge improvements there and I think it should be much cleaner now. Like you mentioned, there is a lot of untyped code and code with types that was added as a best effort without running mypy. I think we will need a really relaxed config or start with only a few SDK files and slowly work up.
It would be awesome if we could get a nicer pattern for tox/mypy/setuptools so any new packages we scaffold will be fully checked from the start
I recently updated mypy to 0.812 version (#1705). Finding code inside packages is much easier right now, so I can take care of running mypy tests for opentelemetry-sdk
. Could you assign me to this issue?
This issue was marked stale due to lack of activity. It will be closed in 30 days.
@dmolenda-sumo Are you still working on this?
Yes, I do. I've been a little busy recently, but I hope to finish this task soon
This issue was marked stale due to lack of activity. It will be closed in 30 days.
@dmolenda-sumo Any progress on this?
It seems like Mypy is expecting there to be an __init__.py
file in the opentelemetry directory
I've experimented with adding an empty __init__.py
file to the root opentelemetry directory and it resolved all of the mypy typing issues that I was having with the opentelemetry libraries.
It seems like you would just need to add it to this location and everything would be fine: https://github.com/open-telemetry/opentelemetry-python/tree/main/opentelemetry-api/src/opentelemetry
Actually, there appear to be quite a few __init__.py
files missing
And I think I'm beginning to see that there might be a problem with the fact that the main module name 'opentelemetry' is used in multiple installation packages, when it probably would have made more sense to have a separate root module name for each package (i.e. 'opentelemetryapi' for -api, 'opentelemetrysdk' for -sdk, etc.)
@aceberle This might help you https://mypy.readthedocs.io/en/stable/command_line.html#cmdoption-mypy-namespace-packages
Hi @srikanthccv, thank you for directing me to that information. So is it expected that anyone pulling opentelemetry into their project will need to run mypy with this --namespace-packages
argument to avoid the import errors?
I noticed that the tox.ini file uses this argument that you pointed out with the mypy command during the automated tests: https://github.com/open-telemetry/opentelemetry-python/blob/95aeeccb59945fee7f8cc40299df12a85ebc5783/tox.ini#L174
Yes
Thank you so much @srikanthccv, I added that to my configuration and now mypy is able to resolve all of the imports from opentelemetry-python!
I"m happy to do a bit of work on this but to be honest I've found working with the opentelemetry repos very hard. I have plenty of experience working with complex setups but I keep finding issues with these repos, like linting seemingly not running properly in CI or errors simply following the CONTRIBUTING.md
instructions. Would one of the maintainers that's familiar with the setup be willing to spend an hour on a call going through things and maybe we can make PRs to fix these sorts of issues or clarify docs where that make sense?
This is one area we all agree that the current experience is really poor. I suspect, that even within the core team, we have found different ways of working around the problems. I want to see this improved. I spent some time in the past to replace our tooling in the hope of a better experience https://github.com/open-telemetry/opentelemetry-python/issues/3260 but that didn't materialize. I will be happy to get on a call but let me throw away my current setup and start fresh to see what are the common issues folks run into and help fix them.
@adriangb @srikanthccv were you able to realize that call? @adriangb do you need further help?
We did have a call, during it we found that formatting / linting wasn’t working at all (or wasn’t working for the sdk package, I don’t remember the exact details, maybe @srikanthccv does) so it was very productive!
That said this issues should remain open since the original request hasn’t been addressed. I personally would like to work on it but have not had time lately.