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

In types.Attributes, change Dict -> Mapping #989

Merged
merged 2 commits into from
Aug 17, 2020

Conversation

aabmass
Copy link
Member

@aabmass aabmass commented Aug 14, 2020

Description

Fixes these kinds of type errors which I'm seeing in the Google cloud exporter:

def usesAttributes(a: types.Attributes) -> None:
    pass

foo: Dict[str, str] = {"key": "value"}

# Causes type error:
# has incompatible type "Dict[str, str]"; expected
# "Optional[Dict[str, Union[str, bool, int, float, Sequence[str], Sequence[bool], Sequence[int], Sequence[float]]]]
usesAttributes(foo)
  • This happens because Dict is invariant (see this mypy FAQ). Using Mapping solves this because it is covariant.
  • This also has the benefit that Mapping is immutable so MyPy will complain if you try and set a value on it.

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

For GoogleCloudPlatform/opentelemetry-operations-python#64

Type of change

Please delete options that are not relevant.

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

How Has This Been Tested?

This is just a type annotation change. All mypy assertions are passing

  • MyPy tox env

Checklist:

(rest are N/A)

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@aabmass aabmass changed the title in types.Attributes, change Dict -> Mapping In types.Attributes, change Dict -> Mapping Aug 14, 2020
@aabmass aabmass marked this pull request as ready for review August 14, 2020 22:33
@aabmass aabmass requested a review from a team August 14, 2020 22:33
Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Nice!

@lzchen lzchen merged commit 1576a0d into open-telemetry:master Aug 17, 2020
srikanthccv pushed a commit to srikanthccv/opentelemetry-python that referenced this pull request Nov 1, 2020
srikanthccv pushed a commit to srikanthccv/opentelemetry-python that referenced this pull request Nov 1, 2020
…metry#989 (open-telemetry#1100)

* docs(batcher): document how to configure custom aggregators open-telemetry#989

* chore: address PR comments

Co-authored-by: Mayur Kale <mayurkale@google.com>
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.

3 participants