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

Typing of AttributeDict return values is broken #1656

Closed
berndbohmeier opened this issue May 19, 2020 · 3 comments · Fixed by #2805
Closed

Typing of AttributeDict return values is broken #1656

berndbohmeier opened this issue May 19, 2020 · 3 comments · Fixed by #2805

Comments

@berndbohmeier
Copy link

berndbohmeier commented May 19, 2020

  • Version: 5.10.0
  • Python: 3.6
  • OS: linux

What was wrong?

Many data like events are returned as an AttributeDict, so that event['event'] and event.event both work. The typings however inherit from TypedDict, for example for EventData, so that only event['event'] correctly typechecks, but not event.event. This is annoying, as perfectly valid code does not pass typechecking.

How can it be fixed?

Somehow allow (similar to the AttributeDict for runtime) for every key to in the TypedDict to bypass typechecking also for the attribute. I sadly do not know how to do this.

@pipermerriam
Copy link
Member

Thanks for bringing this to our attention.

Unfortunately, this is likely complex to fix since TypedDict does not natively provide attribute access. I believe it would require hooking into the mypy plugin APIs.

For v5 I'd be willing to review and merge a fix for this if someone wants to open a PR. I don't think it's something that we'll be prioritizing ourselves.

This gives me more reason to push towards converting all of the return values to use NamedTuple return types instead. This would effectively deprecate event['event'] style access and enshrine event.event.

I believe that we can use the same pattern from eth-account to maintain backwards compatibility with runtime code.

What I'm curious about is whether there are any compelling reasons to stick with dictionary style return types. We've been using NamedTuple return types in our codebases more and more (in places that historically would have often involved a dictionary) and so far I feel the results have been quite positive.

@berndbohmeier
Copy link
Author

Yes, I thought this might be complex. Using NamedTuple would be awesome.

@pipermerriam
Copy link
Member

linking to #1416 and adding "good first issue" even though this is really more of a good 2nd or 3rd issue.

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 a pull request may close this issue.

2 participants