SRserves85 / avro-to-python

Light tool for compiling avro schema files (.avsc) to python classes
MIT License
25 stars 19 forks source link

Add `map` type support inside `union` #8

Closed isenilov closed 2 years ago

isenilov commented 2 years ago

This PR adds support of map type inside union. Schema excerpt that caused the error at https://github.com/SRserves85/avro-to-python/blob/bacb3e6eb8bb09bb0a4702f6a5f48a3ef89e11f2/avro_to_python/utils/avro/types/union.py#L101:

...,
{
  "name": "some_name",
  "type": [
    "null",
    {
      "type": "map",
      "values": "float"
    }
  ],
  "default": null
},
...

@SRserves85 @irux @chasdevs Please let me know if this makes sense so I could add the tests for the case.

isenilov commented 2 years ago

Thank you @chasdevs ! I guess you also need to approve the workflow if it is ok to merge.

chasdevs commented 2 years ago

No problem; would you mind adding a quick test before the merge? If it's an emergency I can approve but want to keep that coverage high.

isenilov commented 2 years ago

Added examples for map in union to the test data. There is still one test that is failing but it seems like it is not related to my changes: FAILED tests/test_paths.py::PathTests::test_get_system_file_path - AssertionError: '/tmp/avro/nest/test2.avsc' != '/private/tmp/avro/nest/test2.avsc' Might be my machine though, I am running on Mac

chasdevs commented 2 years ago

Hmm let me try that out rq. Thanks for adding the test data!

chasdevs commented 2 years ago

Yeah not your branch. Approving! If it fails on that one test we can still merge it in.

isenilov commented 2 years ago

Seems like it works!