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

Union argument leads to error #392

Closed
nerdoc opened this issue Mar 29, 2022 · 6 comments
Closed

Union argument leads to error #392

nerdoc opened this issue Mar 29, 2022 · 6 comments
Assignees

Comments

@nerdoc
Copy link
Contributor

nerdoc commented Mar 29, 2022

When having a Union argument type, or Python's new-style 'or' argument type "|", Unicorn crashes. Given the following code:

class MyComponentView(UnicornView):
    state: Union[bool,None] = None

    # or
    # def my_method(state:  bool | None) -> None
    def my_method(state: Union[bool, None]) -> None:  
        self.state=state

When you cal my_method from the client, Unicorn crashes:

  File "/.../.venv/lib/python3.10/site-packages/django_unicorn/views/action_parsers/call_method.py", line 118, in _call_method_name
    if issubclass(type_hints[argument], Model):
TypeError: issubclass() arg 1 must be a class

This is because argument is UnionType here, says the debugger:
image

So I suppose this could be done better...?

@nerdoc
Copy link
Contributor Author

nerdoc commented Mar 29, 2022

This also fails when using typing.Optional[bool]. Same error.

What works, is either using just def my_method(state:bool) - but if you pass None there, it is not correct when you are a Monk like me.

@adamghill
Copy link
Owner

Oh, I think I need to handle the type annotations a little smarter than I currently do. I'll take a look at this and see how to fix it.

@adamghill adamghill self-assigned this Mar 30, 2022
@nerdoc
Copy link
Contributor Author

nerdoc commented Mar 30, 2022

Seems to be a Python problem which is better handled in py38 - if you want to support py36 onwards, have a look at https://stackoverflow.com/a/45959000/818131

@adamghill
Copy link
Owner

Thanks for that SO answer! That provides the clues that are needed to make this type annotation viable.

@adamghill
Copy link
Owner

Since Unicorn doesn't really use the type annotations here (unlike, say Pydantic) I just skip over this error if it happens. I think that should fix your issue, but let me know if it doesn't in the next release.

@adamghill
Copy link
Owner

Should be fixed in 0.44.1 which I just deployed: https://pypi.org/project/django-unicorn/0.44.1/.

will9288 added a commit to will9288/django-unicorn that referenced this issue May 6, 2024
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