-
Notifications
You must be signed in to change notification settings - Fork 651
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
Fix the py.typed marker file usage #4172
Conversation
As defined in PEP 561, the py.typed marker files for namespace packages should be placed "in the submodules of the namespace". In commit 732ea8a, the markers were added to the subpackage roots, which serves no purpose. This changeset addresses all marker-related issues in this repository. A summary of the changes: - Removed the unnecessary py.typed markers from the subpackage roots. - Added the missing marker to the prometheus exporter. Fixes open-telemetry#4113. - Added the missing marker to opentelemetry._events. - Removed the accidentally reintroduced jaeger exporters, which were originally removed in commit 1625b35.
Aren't we doing the following though? |
Yes, you are following it for the most part. Most of the namespace subpackages have the marker, but there were some exceptions: the prometheus exporter and Perhaps an example will help to clarify my point:
So this PR aims to get rid of the |
@kjagiello thanks for the example. I have a followup question: why isn't |
BTW any chance you can add a test calling pytight to avoid regressions and address #4086? |
I think this is missing a |
One thing that comes to mind is that is that there should probably not be any Beware that this is just my interpretation of the PEP, but I think it kind of makes sense. The PEP is at least clear on having the
Sure, I can give it a try! |
To not expand the scope of this PR way too much, I did not address #4086 fully. Currently, pyright is configured to do only basic checks on I also have noticed that The exporters are currently not covered by pyright. |
Oops, had no idea that the workflows were generated using tox. Pushed a fix after running |
Description
As defined in PEP 561, the
py.typed
marker files for namespace packages should be placed in the submodules of the namespace. In commit 732ea8a, the markers were added to the subpackage roots, which serves no purpose. This changeset addresses all marker-related issues in this repository.A summary of the changes:
py.typed
markers from the subpackage roots.opentelemetry._events
.Type of change
Please delete options that are not relevant.
How Has This Been Tested?
tox -e mypy
opentelemetry-api
,opentelemetry-sdk
andopentelemetry-exporter-prometheus
. Previously pyright complained about the exporter missing the marker.Does This PR Require a Contrib Repo Change?
Checklist: