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

Refactor typing_utils.narrow #352

Merged
merged 7 commits into from
Jul 28, 2022
Merged

Refactor typing_utils.narrow #352

merged 7 commits into from
Jul 28, 2022

Conversation

odashi
Copy link
Contributor

@odashi odashi commented Jul 26, 2022

This pr introduces some small changes in typing_utils.narrow:

  • Write a complete documentation.
  • Use __name__ in the error string rather than the str representation.
  • Change the argument order to narrow(type, obj) to keep consistency with typing.cast

@odashi odashi requested a review from lyuyangh July 26, 2022 19:03
@odashi
Copy link
Contributor Author

odashi commented Jul 27, 2022

Some tricky error occurred around cast, investigating the cause.

@odashi
Copy link
Contributor Author

odashi commented Jul 28, 2022

cast does not work with TypeVar.
python/mypy#6440

As the type inference can be performed correctly without cast (because of the isinstance narrowing), simply avoiding cast may be the right choice for now.

@odashi odashi requested a review from neubig July 28, 2022 04:47
Copy link
Contributor

@neubig neubig left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@odashi odashi merged commit ea07c34 into main Jul 28, 2022
@odashi odashi deleted the downcast branch July 28, 2022 21:37
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