[GitHub] [avro] kojiromike commented on a change in pull request #1167: AVRO-1938: Generate Parsing Canonical Forms of Schema

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

kojiromike commented on a change in pull request #1167:
URL: https://github.com/apache/avro/pull/1167#discussion_r605599414



##########
File path: lang/py/avro/schema.py
##########
@@ -136,7 +147,18 @@ def _is_timezone_aware_datetime(dt):
 # Base Classes
 #
 
-class Schema(abc.ABC):
+class CanonicalPropertiesMixin(object):
+    """A Mixin that provides canonical properties to Schema and Field types."""
+    @property
+    def canonical_properties(self):
+        props = self.props
+        return collections.OrderedDict(

Review comment:
       Should we just use a dictionary here?

##########
File path: lang/py/avro/schema.py
##########
@@ -469,10 +511,19 @@ def __str__(self):
         return json.dumps(self.to_json())
 
     def to_json(self, names=None):
-        if names is None:
-            names = Names()
+        names = names or Names()
+
         to_dump = self.props.copy()
         to_dump['type'] = self.type.to_json(names)
+
+        return to_dump
+
+    def to_canonical_json(self, names=None):
+        names = names or Names()

Review comment:
       At some point (not here) we should consider tightening up these interfaces so we don't have to copy patterns like this into every implementation.

##########
File path: lang/py/avro/schema.py
##########
@@ -291,22 +331,24 @@ def has_name(self, name_attr, space_attr):
 
     def get_name(self, name_attr, space_attr):
         test = Name(name_attr, space_attr, self.default_namespace).fullname
-        if test not in self.names:
-            return None
-        return self.names[test]
+
+        return None if test not in self.names else self.names[test]

Review comment:
       Seems like `self.names.get(test)` should be equivalent.




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