[GitHub] [beam] udim commented on a change in pull request #11939: [BEAM-10197] Support typehints for Python's frozenset

[GitBox <gi...@apache.org>]

udim commented on a change in pull request #11939:
URL: https://github.com/apache/beam/pull/11939#discussion_r436187871



##########
File path: sdks/python/apache_beam/typehints/typehints_test.py
##########
@@ -612,54 +613,70 @@ def test_match_type_variables(self):
                      hint.match_type_variables(typehints.Dict[int, str]))
 
 
-class SetHintTestCase(TypeHintTestCase):
+class BaseSetHintTest:
+  def __init__(self, string_type, py_type, beam_type, *args, **kwargs):

Review comment:
       You should omit `*args, **kwargs` if there aren't any more args allowed. This is the style we follow in this codebase. Here and below as well.
   
   For example: if some code called `BaseSetHintTest('Set', set, typehints.Set, typehints.FrozenSet)` would you rather it be silently ignored or raised as an incorrect number of arguments?

##########
File path: sdks/python/apache_beam/typehints/typehints_test.py
##########
@@ -612,54 +613,70 @@ def test_match_type_variables(self):
                      hint.match_type_variables(typehints.Dict[int, str]))
 
 
-class SetHintTestCase(TypeHintTestCase):
+class BaseSetHintTest:

Review comment:
       If `BaseSetHintTest` inherited from `TypeHintTestCase` you could avoid multiple inheritance in the sub-classes below. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org