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

Use Python syntax for type hints instead of comments #50

Merged
merged 1 commit into from
Mar 16, 2019
Merged

Use Python syntax for type hints instead of comments #50

merged 1 commit into from
Mar 16, 2019

Conversation

wbolster
Copy link
Contributor

@wbolster wbolster commented Mar 8, 2019

This changes all type hints to use modern Python (3.6+) syntax instead
of special comments, but leaves the ‘type: ignore’ comments intact.

Most of these changes are purely mechanical, except for these:

  • It seems the _message_dict can only ever contain str and int keys,
    so make that explicit instead of using typing.Any.

  • The call to datetime.time(**kwargs, tzinfo=None) gives mypy
    warnings which can be avoided by reordering the arguments. Changing
    it to datetime.time(tzinfo=None, **kwargs) will behave the same,
    since kwargs will never contain a tzinfo key, because
    TIME_REGEX does not have a capturing group with that name.

This changes all type hints to use modern Python (3.6+) syntax instead
of special comments, but leaves the ‘type: ignore’ comments intact.

Most of these changes are purely mechanical, except for these:

- It seems the _message_dict can only ever contain str and int keys,
  so make that explicit instead of using `typing.Any`.

- The call to `datetime.time(**kwargs, tzinfo=None)` gives mypy
  warnings which can be avoided by reordering the arguments. Changing
  it to `datetime.time(tzinfo=None, **kwargs)` will behave the same,
  since `kwargs` will never contain a `tzinfo` key, because
  `TIME_REGEX` does not have a capturing group with that name.
@wbolster
Copy link
Contributor Author

ping, any chance to get this merged? the branch got out of date but i just resolved the merge conflicts and it now applies cleanly again.

@tomchristie
Copy link
Member

👍

@tomchristie tomchristie merged commit 0429579 into encode:master Mar 16, 2019
@tomchristie
Copy link
Member

Good stuff, thanks!

@wbolster wbolster deleted the use-modern-typing-syntax branch March 16, 2019 20:03
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.

2 participants