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

Update __all__ to statically define reexports #3143

Merged
merged 10 commits into from
Feb 15, 2023

Conversation

jenshnielsen
Copy link
Contributor

@jenshnielsen jenshnielsen commented Jan 25, 2023

Description

Update all __all__ declarations to statically define all members. This resolves error messages from IDE/static type checkers

Fixes #3141

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Ran tox locally.
No new functionality

This enables static checkers such as pylint to now verify that imported objects are being used
so removed a bunch of now redundant supress comments

Does This PR Require a Contrib Repo Change?

  • No.

Checklist:

This prevents errors from typecheckers since
they can now see that members are explicitly reexported
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jan 25, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: jenshnielsen / name: Jens Hedegaard Nielsen (a95bde9, 937cc8d)

jenshnielsen added a commit to jenshnielsen/opentelemetry.io that referenced this pull request Feb 1, 2023
With the merge of open-telemetry/opentelemetry-python#3143
this work around is no longer required and can be removed from
the docs.
@jenshnielsen jenshnielsen marked this pull request as ready for review February 1, 2023 16:39
@jenshnielsen jenshnielsen requested a review from a team February 1, 2023 16:39
@jenshnielsen
Copy link
Contributor Author

This should be ready for review. I however noticed that the docs build fails.

sphinx.errors.SphinxWarning: /home/jenielse/source/repo/opentelemetry-python/.tox/docs/lib/python3.9/site-packages/opentelemetry/_logs/_internal/__init__.py:docstring of opentelemetry._logs._internal.LoggerProvider.get_logger:28:py:class reference target not found: opentelemetry._logs._internal.Logger

This seems to be that Sphinx cannot find the reference for the Logger class in return types. Not sure why this is happening

Copy link
Member

@aabmass aabmass left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

@srikanthccv
Copy link
Member

Does the docs pass if you move the Logger class to end of the file?

@jenshnielsen
Copy link
Contributor Author

Does the docs pass if you move the Logger class to end of the file?

I will give it a try. I did try on a different branch to upgrade to newest Sphinx and replace autodoc_typehints with the build in sphinx feature. That seems to resolve this issue but triggers others

@srikanthccv
Copy link
Member

Add an entry to the nitpick_ignore list for this if it doesn't go away.

@ocelotl ocelotl merged commit e03650a into open-telemetry:main Feb 15, 2023
@jenshnielsen jenshnielsen deleted the static_all branch February 15, 2023 05:23
jenshnielsen added a commit to jenshnielsen/opentelemetry.io that referenced this pull request Feb 24, 2023
With the merge of open-telemetry/opentelemetry-python#3143
this work around is no longer required and can be removed from
the docs.
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.

Dynamically generated __all__ in opentelemetry sdk does not work with static type checkers
4 participants