godatadriven / pydantic-avro

This library can convert a pydantic class to a avro schema or generate python code from a avro schema.
https://github.com/godatadriven/pydantic-avro
MIT License
60 stars 30 forks source link

Misleading error on `Any` type #100

Closed jspaezp closed 8 months ago

jspaezp commented 8 months ago

Hello there!

I was working with the package and wanted to suggest an improvement on an error message. LMK if you would like a PR for it.

Reproducible example:

import json
from typing import Optional, Any

from pydantic_avro.base import AvroBase

class Foo(AvroBase):
    x: Any
    y: Optional[int] = None

schema_dict: dict = Foo.avro_schema() # This line fails
print(json.dumps(schema_dict, indent=2))

desired_inputs = [
    {
        "x": 1,
        "y": 2,
    },
    {
        "x": 3,
        "y": None,
    },
    {
        "x": 4,
    },
]

for i, desired_input in enumerate(desired_inputs):
    print(f"Test {i}")
    try:
        foo = Foo(**desired_input)
        print("Success!")
    except ValueError as e:
        print("Failure!")

Traceback (most recent call last): File "/Users/redacted2/test2.py", line 12, in schema_dict: dict = Foo.avro_schema() File "/Users/redacted/lib/python3.10/site-packages/pydantic_avro/base.py", line 29, in avro_schema return cls._avro_schema(schema, namespace) File "/Users/redacted/lib/python3.10/site-packages/pydantic_avro/base.py", line 186, in _avro_schema fields = get_fields(schema) File "/Users/redacted/lib/python3.10/site-packages/pydantic_avro/base.py", line 167, in get_fields avro_type_dict = get_type(value) File "/Users/redacted/lib/python3.10/site-packages/pydantic_avro/base.py", line 155, in get_type raise NotImplementedError( NotImplementedError: Type 'None' not support yet, please report this at https://github.com/godatadriven/pydantic-avro/issues

(I am aware that here the fix is that x is of type Any, but on a real project it took me a while to figure that out.)

Desired error

Field X does not have a defined type / Type is none for ....

Any variant of the error that can point to an un-constrained type AND a field name would be nice.

Fix

pydantic_avro/base.py:153

            else:
                # START +++++
                if t is None:
                    raise ValueError(f"Type is None for {value}")

                # END ++++
                raise NotImplementedError(
                    f"Type '{t}' not support yet, "
                    f"please report this at https://github.com/godatadriven/pydantic-avro/issues"
                )
DaanRademaker commented 8 months ago

Hi! Thanks for raising this issue. We would definitely like to implement this change so if you would like to make a pr go for it!

jspaezp commented 8 months ago

https://github.com/godatadriven/pydantic-avro/pull/101

Nice! here it goes! @DaanRademaker

DaanRademaker commented 8 months ago

Thanks again for your contribution! Feel free to open more issues and pull request if you find more things. Closing issue as the changes have been merged.