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

UnicornView._attribute_names() fails in some components because of unresolved type hints #639

Open
pa-hqt opened this issue Jan 4, 2024 · 5 comments
Assignees

Comments

@pa-hqt
Copy link

pa-hqt commented Jan 4, 2024

One of my components crashes on page load due to recent type hint changes in django_unicorn.components.unicorn_view.UnicornView._attribute_names() of version 0.58.0 (commit 8a9448793)

The head of my component looks like

import datetime
from django_unicorn.components import UnicornView
from my_app.models import AggregatSt

class MyComponentView(UnicornView):
    aggregate: AggregatSt = None
    date: datetime.date = None

Exception:

Traceback (most recent call last):
[...]
  File "...\venv\lib\site-packages\django\template\base.py", line 966, in render_annotated
    return self.render(context)
  File "...\venv\lib\site-packages\django_unicorn\templatetags\unicorn.py", line 179, in render
    self.view = UnicornView.create(
  File "<decorator-gen-19>", line 2, in create
  File "...\venv\lib\site-packages\django_unicorn\components\unicorn_view.py", line 895, in create
    component = construct_component(
  File "<decorator-gen-3>", line 2, in construct_component
  File "...\venv\lib\site-packages\django_unicorn\components\unicorn_view.py", line 151, in construct_component
    component = component_class(
  File "...\venv\lib\site-packages\django_unicorn\components\unicorn_view.py", line 245, in __init__
    self._set_caches()
  File "<decorator-gen-5>", line 2, in _set_caches
  File "...\venv\lib\site-packages\django_unicorn\components\unicorn_view.py", line 273, in _set_caches
    self._attribute_names_cache = self._attribute_names()
  File "<decorator-gen-13>", line 2, in _attribute_names
  File "...\venv\lib\site-packages\django_unicorn\components\unicorn_view.py", line 593, in _attribute_names
    for type_hint_attribute_name in get_type_hints(self).keys():
  File "...\venv\lib\site-packages\django_unicorn\typer.py", line 75, in get_type_hints
    type_hints = typing_get_type_hints(obj)
  File "C:\Users\philip.arnold\AppData\Local\Programs\Python\Python310\lib\typing.py", line 1870, in get_type_hints
    value = _eval_type(value, globalns, localns)
  File "C:\Users\philip.arnold\AppData\Local\Programs\Python\Python310\lib\typing.py", line 327, in _eval_type
    return t._evaluate(globalns, localns, recursive_guard)
  File "C:\Users\philip.arnold\AppData\Local\Programs\Python\Python310\lib\typing.py", line 694, in _evaluate
    eval(self.__forward_code__, globalns, localns),
  File "<string>", line 1, in <module>
NameError: name 'AggregatSt' is not defined

In typing.get_type_hints()#L1859 the hints dictionary has following values:

hints = {'aggregate': 'AggregatSt', 'date': 'datetime.date'}

Since the values are strings, they are used to construct ForwardRef, but they cannot be solved, because globalns and localns are empty.

Any ideas, why the imports fail?
May it be a solution to add NameError to catch block in django_unicorn.typer#L81?

(same happens with 'datetime' if I remove the type hint of my custom class 'AggregatSt')

I am using python 3.10, Django 4.2.9 and django-unicorn 0.58.0, here.

@adamghill adamghill self-assigned this Jan 4, 2024
@adamghill
Copy link
Owner

Oh shoot, I'm sorry you ran into this issue. I will release a hot-fix tonight to at least prevent this from crashing your component. Thanks again for the bug report.

adamghill added a commit that referenced this issue Jan 4, 2024
adamghill added a commit that referenced this issue Jan 4, 2024
@adamghill
Copy link
Owner

adamghill commented Jan 4, 2024

Unfortunately, I cannot replicate this using Django 4.2.9 and Python 3.10.12. However, I added a catch for NameError anyway in the hopes that it solves your problem. I also wrote a test to show that the key for the dictionary is a class and not a string for me. 🤷 Maybe you can look at my test and see if it breaks for you? I'm curious what else might be going on.

@adamghill
Copy link
Owner

My fix has been merged into main and 0.58.1 has published (https://pypi.org/project/django-unicorn/). Please let me know if that solves your problem or if you are still running into an issue.

@pa-hqt
Copy link
Author

pa-hqt commented Jan 5, 2024

Thank you very much for providing this hotfix instantly!
Yes, that helps me, because the component can be constructed without exception now.

I do not understand what is different with that component, I descriped above, than others that have similar class attribute definitions and worked without error. Also your new test case does not fail on my maschine. So this seams to be an issue of category "unpredictable".

But inspired by your test case, I wrote some more that pass with 0.58.1 and do not with 0.58.0.
Maybe you should check, if that test expectations match your personal expectation what should happen in a component.

import datetime

from django_unicorn.components import UnicornView
from django_unicorn.typer import get_type_hints


def test_get_type_hints_gh_639_hint():
    class MyComponentView(UnicornView):
        a_date: datetime.date

    expected = {"a_date": datetime.date}
    actual = get_type_hints(MyComponentView(component_name="test", component_id="123"))
    assert actual == expected


def test_get_type_hints_gh_639_hint_with_default():
    class MyComponentView(UnicornView):
        a_date: datetime.date = None

    expected = {"a_date": datetime.date}
    actual = get_type_hints(MyComponentView(component_name="test", component_id="123"))
    assert actual == expected


def test_get_type_hints_gh_639_ref():
    class MyComponentView(UnicornView):
        a_date: "datetime.date"

    expected = {}
    actual = get_type_hints(MyComponentView(component_name="test", component_id="123"))
    assert actual == expected


def test_get_type_hints_gh_639_ref_with_default():
    class MyComponentView(UnicornView):
        a_date: "datetime.date" = None

    expected = {}
    actual = get_type_hints(MyComponentView(component_name="test", component_id="123"))
    assert actual == expected


def test_get_type_hints_gh_639_ref_with_kwarg():
    class MyComponentView(UnicornView):
        a_date: "datetime.date"

    expected = {}
    actual = get_type_hints(MyComponentView(component_name="test", component_id="123", a_date=datetime.date.fromisoformat("2024-01-05")))
    assert actual == expected


def test_component_attribute_names_gh_639_hint():
    class MyComponentView(UnicornView):
        a_date: datetime.date

    expected = ["a_date"]
    actual = MyComponentView(component_name="test", component_id="123")._attribute_names()
    assert actual == expected


def test_component_attribute_names_gh_639_hint_with_default():
    class MyComponentView(UnicornView):
        a_date: datetime.date = None

    expected = ["a_date"]
    actual = MyComponentView(component_name="test", component_id="123")._attribute_names()
    assert actual == expected


def test_component_attribute_names_gh_639_ref():
    class MyComponentView(UnicornView):
        a_date: "datetime.date"

    expected = []
    actual = MyComponentView(component_name="test", component_id="123")._attribute_names()
    assert actual == expected


def test_component_attribute_names_gh_639_ref_with_default():
    class MyComponentView(UnicornView):
        a_date: "datetime.date" = None

    expected = ["a_date"]
    actual = MyComponentView(component_name="test", component_id="123")._attribute_names()
    assert actual == expected


def test_component_attribute_names_gh_639_ref_with_kwarg():
    class MyComponentView(UnicornView):
        a_date: "datetime.date"

    expected = ["a_date"]
    actual = MyComponentView(component_name="test", component_id="123", a_date=datetime.date.fromisoformat("2024-01-05"))._attribute_names()
    assert actual == expected

@adamghill
Copy link
Owner

Thanks for these test cases! Looking through them, I think there are some gaps in how unicorn deals with certain type annotations for sure.

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