python-diskcache icon indicating copy to clipboard operation
python-diskcache copied to clipboard

Implement PEP 562 for Python >= 3.7

Open mondeja opened this issue 2 years ago • 4 comments

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:

image

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:

image

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.

mondeja avatar Dec 17 '22 20:12 mondeja

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

mondeja avatar Dec 19 '22 14:12 mondeja

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.

grantjenks avatar Feb 27 '24 00:02 grantjenks

To be clear, you're asking about a library that supports PEP562 with these kind of exceptions like the djangocache module?

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 __getattr__ and __dir__ functions. I don't think it's worth adding a dependency plus all the performance impact of wrapping the module in a class for that.

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.

mondeja avatar Feb 27 '24 13:02 mondeja

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. However, this will not make IDE work. In order for IDE to work properly, we need to make the following modifications.

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

        return DjangoCache

    error_msg = f"module {__name__!r} has no attribute {attr!r}"
    raise AttributeError(error_msg)

phi-friday avatar Aug 13 '24 05:08 phi-friday