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

Usage with dataclasses and inheritance #375

Open
tuukkamustonen opened this issue Feb 23, 2025 · 16 comments
Open

Usage with dataclasses and inheritance #375

tuukkamustonen opened this issue Feb 23, 2025 · 16 comments
Labels
bug Something isn't working

Comments

@tuukkamustonen
Copy link

tuukkamustonen commented Feb 23, 2025

Hey, seems like a well thought-out project 👍🏻

Though, is it supposed to work with dataclasses and inheritance?

diparent.py:

from __future__ import annotations  # <--- this triggers the problem

from dataclasses import dataclass


class LocalDep:
    pass


@dataclass
class Parent:
    dep1: LocalDep

dichild.py:

from dataclasses import dataclass

from diparent import Parent
from dishka import make_container, Provider, Scope


@dataclass
class Child(Parent):
    pass


provider = Provider(scope=Scope.APP)
provider.provide(Child, recursive=True)
container = make_container(provider)
container.get(Child)

Raises error:

  File "/paaaaaath/lib/python3.12/site-packages/dishka/dependency_source/make_factory.py", line 224, in _make_factory_by_class
    raise NameError(
NameError: Failed to analyze `__main__.Child.__init__`. 
Type 'LocalDep' is not defined

If your are using `if TYPE_CHECKING` to import 'LocalDep' then try removing it. 
Or, create a separate factory with all types imported.

Dropping the from __future__ import annotations in diparent.py removes the error.

(Dataclasses reduce the __init__ boilerplate greatly, wouldn't want to re-declare parent's dependencies always when inheriting.)

@Tishka17
Copy link
Member

I strictly recommend avoid lazy annotations as much as possible. Though it looks more like a python bug. We need to investigate what can be done here

@Niccolum
Copy link

@tuukkamustonen hello

Can you add your traceback? And, if it possible, don't clear real dependency name

@Tishka17 Tishka17 added the bug Something isn't working label Feb 24, 2025
@tuukkamustonen
Copy link
Author

lazy annotations

What do you refer to by that?

Can you add your traceback? And, if it possible, don't clear real dependency name

I'll try to add a real reproducible example.

@Tishka17
Copy link
Member

lazy annotations

What do you refer to by that?

I refer to from __future__ import annotations or using strings as annotations.

@tuukkamustonen
Copy link
Author

tuukkamustonen commented Feb 24, 2025

I refer to from __future__ import annotations or using strings as annotations.

Okay, neither of those are in play here. It's just inheriting the dep1 from a parent class - the type for it is not explicitly imported in the child classes file, so Dishka throws an error.

Tried to create a minimal reproducible example, but eh eh, but couldn't get it fail. Need to double check my previous code, if I missed anything.

Will get back.

@tuukkamustonen
Copy link
Author

@Tishka17 @Niccolum Updated the ticket description, to indicate when it happens.

@Tishka17
Copy link
Member

I tried simple thing

print(get_type_hints(Child))
print(get_type_hints(Child.__init__))

First line works ok, but second fails.

@tuukkamustonen
Copy link
Author

tuukkamustonen commented Feb 24, 2025

So the issue is deep down in Python/dataclasses/typing itself... 🤔

@tuukkamustonen
Copy link
Author

Seems to be python/cpython#89687

@Tishka17
Copy link
Member

Tishka17 commented Feb 24, 2025

From my perspective, the bug is in dataclass, which doesn't keep reference to original module while setting __init__ hints. We'll think about workarounds here, but my general recommendation is still the same - avoid using from __future__ import annotations as much as possible

@tuukkamustonen
Copy link
Author

tuukkamustonen commented Feb 24, 2025

I would assume from __future__ import annotations usage is pretty wide, though, as it allows you to avoid cyclical imports, optimize loading times, avoids having to define types as strings, etc. Uh, I'm mixing with if TYPE_CHECKING blocks.

I know it's a controversial topic (at least historically), but yeah, probably quite widely used. So, if there's a way to support this, do consider. Although, this is a bit of a corner case (ie. dataclasses + future annotations + cross-modules).

@Tishka17
Copy link
Member

Tishka17 commented Feb 24, 2025

as it allows you to avoid some cyclical imports, optimize loading times, avoids having to define types as strings, etc.

It doesn't allow to avoid cyclical imports, it masquerades them. Al that you mention is potentially a code smell which should be fixed.

Anyway, I've create an issue python/cpython#130506

@tuukkamustonen
Copy link
Author

tuukkamustonen commented Feb 24, 2025

There was a suggestion to use inspect.get_annotations(), instead.

That indeed works for the example case here.

@Tishka17
Copy link
Member

I do not see how to use inspect.get_annotations() here. Do you have a working proposal?

@tuukkamustonen
Copy link
Author

Unfortunately, no. Looks like it's dumb as you say, and only returns the object name and not the full qualname (so inspect.get_annotations(..., eval_str=True) fails to resolve the type) 🤷‍♂

@tuukkamustonen
Copy link
Author

tuukkamustonen commented Feb 26, 2025

Though, could you use a combination of get_type_hints() and get_annotations()?

get_type_hints(Child) works and returns (all the object's) types.

get_annotations(Child.__init__) gives list of arguments the __init__ takes. Or could use inspect.signature() and friends.

Could it combine the information for the two?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants