WIP: fix json encoding#1
WIP: fix json encoding#1mhoffm-aiven wants to merge 3 commits intomhoffm-fix-python-json-encodingfrom
Conversation
… compound objects
…; start refactoring deocder and DatumReader
| class DatumReader: | ||
| """Deserialize Avro-encoded data into a Python data structure.""" | ||
|
|
||
| _writers_schema: Optional[avro.schema.Schema] |
There was a problem hiding this comment.
I know it's copied but I think it's wrong because it defines type for a class attribute but it's an object attribute. Probably should be removed.
There is the same pattern in multiple places in this file.
There was a problem hiding this comment.
This is totally fine:
You can optionally declare instance variables in the class body
https://mypy.readthedocs.io/en/stable/cheat_sheet_py3.html#classes
There was a problem hiding this comment.
Looks really confusing to me. Especially that the example is wrong here:
class MyClass:
# This is an instance variable with a default value
charge_percent: int = 100It's still a class variable. It works here because the value is immutable. It won't work as expected if it will be something like items: List[int] = [100].
I wouldn't support that kind of confusion.
There was a problem hiding this comment.
It's still a class variable. It works here because the value is immutable. It won't work as expected if it will be something like items: List[int] = [100].
There is not assignment here, it is just the type declaration PEP-0526.
You're totally correct that a mutable value things could be broken though.
| raise avro.errors.SchemaResolutionException("Schemas do not match.", writers_schema, readers_schema) | ||
|
|
||
| if writers_schema.type == "null": | ||
| return None |
There was a problem hiding this comment.
Even while returning None an encoder can potentially read something from the stream
| return None | |
| return decoder.read_null() |
3 participants
This PR fixes https://issues.apache.org/jira/browse/AVRO-1291 by adding proper json encoding capabilities to the python library.