-
Notifications
You must be signed in to change notification settings - Fork 137
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
Implement PEP 562 for Python >= 3.7 #269
base: master
Are you sure you want to change the base?
Conversation
Current workaround: import os
import importlib.util
_diskcache_init_path = importlib.util.find_spec("diskcache").origin
_diskcache_core_spec = importlib.util.spec_from_file_location(
"diskcache.core",
os.path.join(
os.path.dirname(_diskcache_init_path),
"core.py",
)
)
_diskcache_core = importlib.util.module_from_spec(
_diskcache_core_spec,
)
_diskcache_core_spec.loader.exec_module(_diskcache_core)
Cache = _diskcache_core.Cache |
Is there any way to refactor this code to a library that implements PEP 562 and could be used instead? As it it is currently, it's helpful but kinda inscrutable. |
To be clear, you're asking about a library that supports PEP562 with these kind of exceptions like the Exists pep562 but unfortunately does not support optional imports like the django one that this library uses, so in the end you still need to create the Perhaps the most comfortable solution would be to force users to import the Django cache from their own module, which would be a breaking change: from diskcache.djangocache import DjangoCache Another solution could be to stop supporting Python3.7, which has reached his EOL, so the code could be much more simplified. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like you could do:
def __getattr__(attr):
if attr == "DjangoCache":
from .djangocache import DjangoCache
obj = DjangoCache
else:
raise AttributeError(f"module {__name__!r} has no attribute {attr!r}")
globals()[attr] = obj
return obj
and it would be simpler + allow IDEs and type checkers to still resolve symbols
(Separate module seems like a good alternative idea, at cost of breaking change)
I think this is better too. from typing import TYPE_CHECKING, Any
if TYPE_CHECKING:
from diskcache.djangocache import DjangoCache # noqa: F401
def __getattr__(attr: str) -> Any:
if attr == "DjangoCache":
from diskcache.djangocache import DjangoCache
globals()[attr] = DjangoCache
return DjangoCache
error_msg = f"module {__name__!r} has no attribute {attr!r}"
raise AttributeError(error_msg) |
Currently when importing whatever object from diskcache, if
django
is installed it is imported. You can see it in the next image generated by pyinstrument:To avoid that, this PR implements PEP 562 on the __init__.py file for Python 3.7 onwards. After it, when importing whatever object, like with
from diskcache import Cache
for example, django is not imported:According to my benchmarks, this avoids ~100ms of initialization time on all imports except
DjangoCache
on Python3.8. Especially useful in CLI programs that don't need django and should start as fast as possible.