hynek/svcs

`get_type_hints` fails on any of the `svcs` types

guacs opened this issue · 10 comments

A minimal reproduction:

from svcs import Registry
from typing import get_type_hints

get_type_hints(Registry)

The reason for this is the changing of the module of the different types here. This fails since get_type_hints depends on the __module__ of the given object to find the global namespace in order to resolve stringified annotations.

EDIT: Finished writing the description since I hit enter by accident before finishing the writing the issue descritpion.

Does this mean we can have either nice names or a working get_type_hints? :-/

Unfortunately, I think so. Or maybe all the types used in the annotations of the different svcs types could be included in svcs.__init__. That might work (haven't tested it) but that feels.... wrong.

Does this mean we can have either nice names or a working get_type_hints? :-/

Removing __future__.annotations from the private modules might have also been an approach to this, that way those types wouldn't have to be resolved from their string names when get_type_hints() is called... I think! 😬

Does this mean we can have either nice names or a working get_type_hints? :-/

Removing __future__.annotations from the private modules might have also been an approach to this, that way those types wouldn't have to be resolved from their string names when get_type_hints() is called... I think! 😬

How would they be resolved then?

Does this mean we can have either nice names or a working get_type_hints? :-/

Removing __future__.annotations from the private modules might have also been an approach to this, that way those types wouldn't have to be resolved from their string names when get_type_hints() is called... I think! 😬

How would they be resolved then?

This would mean that they wouldn't be strings in the type's __annotations__ attribute, they'd be objects so they wouldn't need to be resolved.

>>> from __future__ import annotations
>>> class A:
...   a: int
... 
>>> A.__annotations__
{'a': 'int'}

vs

>>> class A:
...   a: int
... 
>>> A.__annotations__
{'a': <class 'int'>}

But then the new-style annotations would explode on import.

But then the new-style annotations would explode on import.

That's right - however, you need to use annotations compatible with the min supported version of your library if you want to support get_type_hints(), even if you use __future__.annotations.

E.g.,

from __future__ import annotations

class Thing:
  attrib: int | str

This is fine for static type checking on any python version, but if someone passes Thing to get_type_hints() while running on 3.8 or 3.9 it will blow up because the string "int | str" cannot be eval'd on those versions.

Well, yes that's the whole problem. :)

Not using __future__.annotations would "just" pull the agreed-on problem to the front. The current state is that everything works fine on 3.8/3.9 unless you run get_type_hints(). Removing __future__.annotations would break it wholesale.

The end solution would be to rewrite all type hints either way.

The end solution would be to rewrite all type hints either way.

That's right.

The point I'm trying to make is that you can have nice type names, and working get_type_hints (on all supported py versions) - but you have to sacrifice modern type annotations.

Also, you can have modern type annotation syntax and working get_type_hints on the py versions that support the annotation syntax you've used, but you have to sacrifice the nice type names for this.

Hope I'm not coming across argumentative, just want to make sure I've conveyed my point properly (something I struggle with). I just came across this when researching a bug report, and the changes you've already made will have resolved that.

Cheers :)

Right! I'm just a bit afraid it might have more unexpected side-effects, so I'd rather compromise on beauty. :)