Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@extend_schema_field type hint does not accept simple python types #1212

Open
Samuel-Therrien-Beslogic opened this issue Mar 23, 2024 · 5 comments

Comments

@Samuel-Therrien-Beslogic
Copy link

Samuel-Therrien-Beslogic commented Mar 23, 2024

Describe the bug
https://drf-spectacular.readthedocs.io/en/latest/customization.html#step-3-extend-schema-field-and-type-hints states that

In case of basic types (e.g. str, int, etc.) a type hint is already sufficient.

Which seems correct. However, the type hint does no represent that (see screenshot)

To Reproduce
extend_schema_field(list[str])
image (1)

Example from source code: https://github.com/BesLogic/releaf-canopeum/blob/0f327ab8769fefc188be26f25ccd5eeeb331cfda/canopeum_backend/canopeum_backend/serializers.py#L142

Expected behavior
Supported simple python types to be allowed (type[int], type[str], type[list], etc.)
Even better if the list type is recursive (something like _ManyFieldTypes = type[list[_SingleFieldTypes] | _ManyFieldTypes], haven't tested, just writing off the top of my head), but list[Any] is sufficient for our needs


CC @theo-auffeuvre & @NicolasDontigny for visibility on this

@tfranzel
Copy link
Owner

tfranzel commented Mar 23, 2024

The type hint does not reflect that because list[..] was not supported there.

We reflected on that deliberate choice and reversed it. list and other higher order hints were added here: #1181
Please note that this change is not yet released.

But you actually raise a point here, because I think we have not updated the allowed types in the hints accordingly.

@Samuel-Therrien-Beslogic
Copy link
Author

(I've updated the original description to add the link to our repo where this is used)

@tfranzel
Copy link
Owner

Scratch what I said, it was wrong. The feature I was referring to is basically the same thing, but applies somewhere else (not@extend_schema_field). Shouldn't answer issues that late 😄

Okay so the type hint does not cover that, true, but it was not necessary because you are supposed write it normally for regular types.

    def get_sponsors(self, obj) -> list[str]:
        return self.context.get("sponsors")

should do just fine. The decorator is just for cases where an actual type hint (->) would be technically wrong and linters would complain (like with WidgetSerializer or OpenApiTypes.EMAIL).

@tfranzel
Copy link
Owner

tfranzel commented Mar 31, 2024

Added a fix to fully support regular as well as higher order types (union, dict, list).

The hint now also includes basic types (str, int, ...). Unfortunately, it is tricky to create a type hint for higher order types that works with all supported py version (3.7-3.12) and at the same time satisfy mypy. If someone has a good idea, feel free to submit a PR.

So I would recommend to add a type: ignore for those cases in the meantime. The functionality itself is there and it works.

@Samuel-Therrien-Beslogic
Copy link
Author

Samuel-Therrien-Beslogic commented Apr 2, 2024

    def get_sponsors(self, obj) -> list[str]:
        return self.context.get("sponsors")

should do just fine. The decorator is just for cases where an actual type hint (->) would be technically wrong and linters would complain (like with WidgetSerializer or OpenApiTypes.EMAIL).

I see I forgot to mention why we even added extend_schema_field in the first place in my example:
The generated OpenAPI schema resulted in a string rather than a string[], hence we ended up having to explicitly add extend_schema_field. Even though the result of self.context.get("sponsors") was indeed a list[str]

Similarly with get_widget below in my given example, many=True wasn't automatically detected w/o the decorator.

If you're telling me it should've worked as-is w/o the decorator, I can open a different issue for that.

This was done in the context of a weekend hackathon where a handful of us had never even touched django before. So whatever worked quickly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants