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

DEPS: clarifying setuptools's status as a dependency #41815

Closed
fangchenli opened this issue Jun 4, 2021 · 10 comments · Fixed by #42410
Closed

DEPS: clarifying setuptools's status as a dependency #41815

fangchenli opened this issue Jun 4, 2021 · 10 comments · Fixed by #42410
Labels
Dependencies Required and optional dependencies
Milestone

Comments

@fangchenli
Copy link
Member

fangchenli commented Jun 4, 2021

In install.rst, we list setuptools as a runtime dependency. If that is the case, we don't need to vendor packaging #41199 since it's vendored by pkg_resources, which is part of setuptools. And we need to update setup.cfg and README.md to reflect this.

If setuptools is not a runtime dependency, vendoring packaging is necessary. And, we need to list it as a test and optional dependency because we are using pkg_resources in pandas/plotting/_core.py and pandas/tests/plotting/test_backend.py #41503. An alternative of pkg_resources for accessing entry points is importlib.metadata, which is only available after Python 3.8. And importlib.metadata is the suggested way to do this according to https://setuptools.readthedocs.io/en/latest/pkg_resources.html.

@fangchenli fangchenli added Needs Discussion Requires discussion from core team before further action Dependencies Required and optional dependencies labels Jun 4, 2021
@jreback
Copy link
Contributor

jreback commented Jun 4, 2021

setuptools is needed and we should list it

@lithomas1 lithomas1 removed the Needs Discussion Requires discussion from core team before further action label Jun 5, 2021
@lithomas1 lithomas1 added this to the 1.3 milestone Jun 5, 2021
@jorisvandenbossche
Copy link
Member

Setuptools (pkg_resources) is not a required dependency, only an optional one when using plotting backends (it's not even needed when using the default matplotlib backend).
So I don't think we should list it as required dependency.

An alternative of pkg_resources for accessing entry points is importlib.metadata, which is only available after Python 3.8. And importlib.metadata is the suggested way to do this according to https://setuptools.readthedocs.io/en/latest/pkg_resources.html.

Somewhat a separate issue from listing setuptools as dependency or not, but I think we should indeed look into replacing pkg_resources (probably easier to do once we only support Python 3.8+ #41678)

@simonjayhawkins
Copy link
Member

Setuptools (pkg_resources) is not a required dependency, only an optional one when using plotting backends (it's not even needed when using the default matplotlib backend).
So I don't think we should list it as required dependency.

An alternative of pkg_resources for accessing entry points is importlib.metadata, which is only available after Python 3.8. And importlib.metadata is the suggested way to do this according to https://setuptools.readthedocs.io/en/latest/pkg_resources.html.

Somewhat a separate issue from listing setuptools as dependency or not, but I think we should indeed look into replacing pkg_resources (probably easier to do once we only support Python 3.8+ #41678)

maybe open a new issue and we can close this one now that #41818 is merged.

@simonjayhawkins simonjayhawkins removed this from the 1.3 milestone Jun 8, 2021
@jreback jreback added this to the 1.3 milestone Jun 14, 2021
@jreback jreback reopened this Jun 14, 2021
@jreback
Copy link
Contributor

jreback commented Jun 14, 2021

setup tools is an undeclared dependency now

it should be an official runtime one

@jreback jreback added the Blocker Blocking issue or pull request for an upcoming release label Jun 14, 2021
@jorisvandenbossche
Copy link
Member

There is a difference between "optional" and "required" dependencies. We have lots of optional dependencies that are not declared in install_requires. And in practice, setuptools always has been an optional dependency (not required for importing pandas / normal usage, only required when actually using one of the non-default plotting backends).

It's true that we don't provide a proper error message about "pkg_resources" (setuptools) being required for using a plotting backend (as we do for other optional dependencies), but give a plain import error instead. This certainly gives the impression we assume the library is always present, but giving a more informative import error message is also something we can easily fix.

@simonjayhawkins
Copy link
Member

removing the blocker label since #42006 fixed the import issue.

@simonjayhawkins simonjayhawkins removed the Blocker Blocking issue or pull request for an upcoming release label Jun 29, 2021
@simonjayhawkins
Copy link
Member

but giving a more informative import error message is also something we can easily fix.

is that now the only outstanding actionable item here? moving to 1.3.1

@jreback
Copy link
Contributor

jreback commented Jul 6, 2021

actually i think prob ok for 1.3.x to close this (as #42410) is going to remove the need for this in 3.8

@jreback
Copy link
Contributor

jreback commented Jul 6, 2021

we merged the update for 1.4 to remove the need for setuptools entirely, so not merging this to 1.3.x unless strong desire (as efffectively by not merging we are not changing 1.3 vs 1.2 at all w.r.t. requirements).

@jorisvandenbossche
Copy link
Member

Sounds good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dependencies Required and optional dependencies
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants