Add support for collections.abc.Collection type hints#29272
Add support for collections.abc.Collection type hints#29272jrmccluskey merged 5 commits intoapache:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #29272 +/- ##
=======================================
Coverage 38.30% 38.30%
=======================================
Files 690 690
Lines 102043 102072 +29
=======================================
+ Hits 39085 39097 +12
- Misses 61375 61392 +17
Partials 1583 1583
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today! |
|
Assigning reviewers. If you would like to opt out of this review, comment R: @johnjcasey added as fallback since no labels match configuration Available commands:
The PR bot will only process comments in the main thread (not review comments). |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control |
damccorm
left a comment
There was a problem hiding this comment.
Generally LGTM, had a couple minor questions/comments
| is_consistent_with(elem, self.inner_type) | ||
| for elem in sub.tuple_types) | ||
| elif not isinstance(sub, TypeConstraint): | ||
| if getattr(sub, '__origin__', None) is not None: |
There was a problem hiding this comment.
Why do we need this check? I would've assumed issubclass would take care of it
There was a problem hiding this comment.
Parameterized generics and issubclass do not get along. If you pass along a parameterized type (something like issubclass(set[int], collections.abc.Collection)) you actually get an error that the first arg has to be a class. This is why the internal checking for these containers separates out the container type from the type it wraps and checks them separately.
Adds a new internal Beam representation for type hints that are exactly
collections.abc.Collectionstypes. Allows for more general type hinting guarantees.Part of #29135
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.