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

Check Iterator[Span] return type annotations #219

Closed
Oberon00 opened this issue Oct 15, 2019 · 5 comments
Closed

Check Iterator[Span] return type annotations #219

Oberon00 opened this issue Oct 15, 2019 · 5 comments
Labels
api Affects the API package. good first issue Good first issue help wanted release:after-ga To be resolved after GA release

Comments

@Oberon00
Copy link
Member

Oberon00 commented Oct 15, 2019

From #198 (comment)

A unit test should be added to the API package that verifies one can properly use the Spans returned by various APIs as Iterator[Span] without mypy complaining.

@carlosalberto
Copy link
Contributor

I started looking into this - however, even changing the signature to -> typing.ContextManager["Span"] created a rain of errors (which, I guess, we can tell mypy to ignore), i.e.

__init__.py:243: error: "bool" is invalid as return type for "__exit__" that always returns False
__init__.py:243: note: Use "typing_extensions.Literal[False]" as the return type or change it to "None"

There's also the question of how to handle the version check for typing.ContextManager (which, I think, can be simply done by checking the Python version).

@carlosalberto
Copy link
Contributor

(that is, opposed to simply doing a try... except around importing it ;) cc @Oberon00 )

@Oberon00
Copy link
Member Author

I think the rain of errors ⛈ was fixed in #229.

But about the try/except ImportError: I thought that was a common idiom in Python, e.g. for 2/3 compatibility?

@Oberon00 Oberon00 added api Affects the API package. build & infra Issues related to build & infrastructure. labels Oct 28, 2019
@c24t c24t assigned Oberon00 and unassigned carlosalberto Oct 31, 2019
@c24t c24t added this to the Alpha v0.3 milestone Oct 31, 2019
@c24t c24t removed this from the Alpha v0.3 milestone Dec 5, 2019
@codeboten codeboten added the release:after-ga To be resolved after GA release label Sep 24, 2020
@codeboten codeboten added help wanted good first issue Good first issue and removed build & infra Issues related to build & infrastructure. labels Nov 19, 2020
@Oberon00 Oberon00 removed their assignment Jan 15, 2021
@Oberon00
Copy link
Member Author

I noticed this was still assigned to me -- unassigned, as I don't actually have this on my TODO list.

@codeboten
Copy link
Contributor

Closing for now due to inactivity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Affects the API package. good first issue Good first issue help wanted release:after-ga To be resolved after GA release
Projects
None yet
Development

No branches or pull requests

4 participants