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

Remove setuptools from core dependencies #2350

Closed
ZiruiXuQB opened this issue Feb 21, 2023 · 14 comments · Fixed by #3437
Closed

Remove setuptools from core dependencies #2350

ZiruiXuQB opened this issue Feb 21, 2023 · 14 comments · Fixed by #3437
Assignees
Labels
Issue: Feature Request New feature or improvement to existing feature

Comments

@ZiruiXuQB
Copy link

Description

I'm always frustrated when pip install kedro upgrades the versions of setuptools, wheel and pip.

Context

Usually I would not expect pip install to upgrade these packages unless I explicitly run python3 -m pip install --upgrade pip setuptools wheel.

The behaviour is even more surprising as kedro does not give a first impression of a package that is related to packaging or dependency management.

Possible Implementation

  • wheel and pip dependencies will probably be excluded when pip-tools is removed from the core dependencies (by deprecating kedro build-reqs)
  • setuptools can be moved to an optional dependency, kedro[package] to support the functionality of kedro package

Possible Alternatives

n/a

@ZiruiXuQB ZiruiXuQB added the Issue: Feature Request New feature or improvement to existing feature label Feb 21, 2023
@datajoely
Copy link
Contributor

super reasonable suggestion @ZiruiXuQB - I'm actually amazed this hasn't been suggested before!

@deepyaman
Copy link
Member

I wouldn't move setuptools to an optional dependency, but rather remove it altogether. Instead of invoking setup.py directly, it's preferable to use python -m build (as of PEP 517, PEP 518). However, I suppose then build becomes a dependency...

Alternatively, you could just deprecate the kedro package command, because all it is is a thin wrapper on top of a setup.py invocation. I think I'd recommend this, since Kedro went through the exercise of evaluating CLI command usage and deprecating other thin wrappers; not sure why kedro package stats don't show up in that report.

Somewhat relevant (but not a solution): #2280

@noklam
Copy link
Contributor

noklam commented Mar 10, 2023

kedro package is just updated with the packaged config, which may improve the usage(?) @deepyaman

I am adding this to the inbox to make sure we discussed it in backlog grooming. kedro[pacakge] sounds reasonable to me, does micropkg also requires this? I haven't checked but probably yes?

@AhdraMeraliQB AhdraMeraliQB added the Community Issue/PR opened by the open-source community label Mar 21, 2023
@noklam
Copy link
Contributor

noklam commented Apr 6, 2023

Pinging @astrojuanlu

@astrojuanlu
Copy link
Member

Now that I've been summoned 👹 let me just note that in the next pip release, if wheel is not installed and the project does not have a pyproject.toml, pip will not try setup.py install anymore. See pypa/pip#11874 (and related: pypa/pip#11860)

So, I think completing https://github.com/kedro-org/kedro/milestone/36 is becoming more urgent, and after that we could consider this issue.

@datajoely
Copy link
Contributor

Fantastic push astro-👹

@merelcht merelcht removed the Community Issue/PR opened by the open-source community label Apr 11, 2023
@astrojuanlu
Copy link
Member

This is blocked by migrating to pyproject.toml (see https://github.com/kedro-org/kedro/milestone/36)

@noklam
Copy link
Contributor

noklam commented Aug 3, 2023

@astrojuanlu Can we close this already? I think for kedro it is move to pyproject.toml so these are only build requirements now?

@astrojuanlu
Copy link
Member

kedro still has a dependency on pip-tools and setuptools:

kedro/pyproject.toml

Lines 31 to 36 in 664411d

"pip-tools>=6.5,<8",
"pluggy~=1.0",
"PyYAML>=4.2, <7.0",
"rich>=12.0, <14.0",
"rope>=0.21, <2.0", # subject to LGPLv3 license
"setuptools>=65.5.1",

IIUC, when we remove kedro build-reqs part of this issue will fix itself.

OTOH, kedro package now does not depend on setuptools anymore, and build does not depend on it either:

https://github.com/pypa/build/blob/381ce789e1662e2e16b0ca4be520cdcf7464f5a3/pyproject.toml#L34

I was under the impression that we needed to wait on #2853 to remove our direct dependency on setuptools, but maybe we can proceed already. That would fix the other half of the issue.

@astrojuanlu
Copy link
Member

Double check micropkg though, I recall we use setuptools package discovery. The structure of kedro micropkg package could be changed into an src-layout to avoid this, but it would be a breaking change. If that's needed to close this issue though, maybe let's do it ahead of 0.19.

@astrojuanlu
Copy link
Member

This is the code that uses setuptools on runtime:

# Project name will be `my-pipeline` even if `pyproject.toml` says `my_pipeline`
# because standards mandate normalization of names for comparison,
# see https://packaging.python.org/en/latest/specifications/core-metadata/#name
# The proper way to get it would be
# project_name = library_meta.get("Name")
# However, the rest of the code expects the non-normalized package name,
# so we have to find it.
packages = [
package
for package in FlatLayoutPackageFinder().find(project_root_dir)
if "." not in package
]

@astrojuanlu
Copy link
Member

Ideas to remove dependency on setuptools:

  • In pyproject.toml, explicitly declare the packages ("packages = ...") instead of using the autodiscovery ("tool.setuptools.packages.find").
  • Alternatively, in the pyproject.toml generated by micropkg, add a [tool.kedro] section that contains package_name, so we don't have to guess it.

@merelcht
Copy link
Member

Only removing setuptools is left to do.

@DimedS DimedS self-assigned this Dec 15, 2023
@DimedS DimedS linked a pull request Dec 18, 2023 that will close this issue
7 tasks
@astrojuanlu astrojuanlu changed the title Remove setuptools, wheel and pip from core dependencies Remove setuptools from core dependencies Dec 18, 2023
@astrojuanlu
Copy link
Member

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue: Feature Request New feature or improvement to existing feature
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

9 participants