Abacn commented on code in PR #22561:
URL: https://github.com/apache/beam/pull/22561#discussion_r956433523


##########
sdks/python/apache_beam/typehints/schemas.py:
##########
@@ -587,6 +588,7 @@ def to_language_type(value):
   def register_logical_type(cls, logical_type_cls):
     """Register an implementation of LogicalType."""
     cls._known_logical_types.add(logical_type_cls.urn(), logical_type_cls)
+    return logical_type_cls

Review Comment:
   These place were some minor bugs. If not return logical_type_cls, class 
decorated with `@register_logical_type` cannot be imported (will be None) and 
docstring is also missing from pydoc; signatures of `to_representation_type` 
and `to_language_type` should have a value parameter as its child classes. 
Fixed here.



##########
sdks/python/apache_beam/typehints/schemas.py:
##########
@@ -587,6 +588,7 @@ def to_language_type(value):
   def register_logical_type(cls, logical_type_cls):
     """Register an implementation of LogicalType."""
     cls._known_logical_types.add(logical_type_cls.urn(), logical_type_cls)
+    return logical_type_cls

Review Comment:
   These places were some minor bugs. If not return logical_type_cls, class 
decorated with `@register_logical_type` cannot be imported (will be None) and 
docstring is also missing from pydoc; signatures of `to_representation_type` 
and `to_language_type` should have a value parameter as its child classes. 
Fixed here.



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to