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

Do not require on pkg_resources as it drastically increases import time. #5676

Closed
marscher opened this issue Aug 5, 2021 · 2 comments · Fixed by #5845
Closed

Do not require on pkg_resources as it drastically increases import time. #5676

marscher opened this issue Aug 5, 2021 · 2 comments · Fixed by #5845

Comments

@marscher
Copy link
Contributor

marscher commented Aug 5, 2021

Is your feature request related to a problem? Please describe.
Most of the import time of xarray is wasted by importing pkg_resources. As far as I could see, the module is used to set the version number of xarray. So importing xarray takes like 0.275s (old machine) and of this 0.17s are just spent in importing pkg_resources.

Describe the solution you'd like
There are better approaches to set the version number than to use this heavy weight module. For instance setuptools_scm.

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Additional context
xarray-version 0.19.0

@benbovy
Copy link
Member

benbovy commented Aug 5, 2021

There are better approaches to set the version number than to use this heavy weight module. For instance setuptools_scm.

Looks like it is still required to retrieve the version number at runtime for python versions < 3.8, unless we agree to temporarily depend on the importlib_metada backport.

As far as I could see, the module is used to set the version number of xarray

It is also used for the backend entry-points and for loading a couple of static css/html files for the html reprs. I don't know if there are better approaches for this, though.

@keewis
Copy link
Collaborator

keewis commented Aug 5, 2021

we had a lengthy discussion about a related topic in #4295. Long story short: we will be able to switch to importlib.metadata once we can drop support for py37 (and we can already use importlib.resources for the CSS and HTML files, feel free to send in a PR for that).

For the backends, we could use the separate entrypoints library (which I believe is faster), but as far as I can tell we will only make that switch once we can drop the dependency on pkg_resources entirely.

For reference, NEP29 allows dropping support for py37 after Dec 26.

marscher added a commit to marscher/xarray that referenced this issue Oct 7, 2021
Fixes pydata#5676

Now depends on importlib-metadata for Python major version < 3.8
cheginit pushed a commit to hyriver/async-retriever that referenced this issue Nov 2, 2021
Illviljan added a commit that referenced this issue Jan 11, 2022
* remove requirement for setuptools.pkg_resources

Fixes #5676

Now depends on importlib-metadata for Python major version < 3.8

* doh

* doh2

* doh3, no reraise for fallback version number

* black formatting

* ordering

* all this lifetime wasted by fucking linting tools

* precommit

* remove python 3.7 as min supported version

* remove 3.7 from min_deps

* Update ci-additional.yaml

* test increasing dask

* test increasing numba

* Update ci-additional.yaml

* remove setuptools from CI

* undo dask/numba tests

* Remove importlib_metadata

* remove 3.7 compats

* Remove 3.7 compat

* remove sys

* Update options.py

* Update whats-new.rst

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Run flaky/all but dask with latest python version

* fix all_but_dask file as well

* Update dataarray.py

* Update dataarray.py

* Update doc/whats-new.rst

Co-authored-by: keewis <[email protected]>

* update the install guide

* remove py37 only dependencies

* don't install importlib-metadata and typing_extensions into min-deps envs

* update the requirements file

* add back the optional typing_extensions dependency

Co-authored-by: Martin K. Scherer <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: keewis <[email protected]>
Co-authored-by: Keewis <[email protected]>
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

Successfully merging a pull request may close this issue.

3 participants