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

Migrate kedro's setup.py to pyproject.toml #2152

Closed
idanov opened this issue Dec 21, 2022 · 8 comments
Closed

Migrate kedro's setup.py to pyproject.toml #2152

idanov opened this issue Dec 21, 2022 · 8 comments
Labels
Component: DevSetup Issue/PR that addresses technical setup of the project repository

Comments

@idanov
Copy link
Member

idanov commented Dec 21, 2022

Description

The Python community is slowly moving to pyproject.toml as a package definition format and away from big and verbose setup.py files. Our Kedro package setup is still exclusively done in setup.py, with some parts in setup.cfg and pyproject.toml. We should move entirely to a pyproject.toml Python package definition for future-proofing our package setup.

Context

Adhering to the Python community standards is important in order to make it easier for future contributors to Kedro.

Possible Implementation

We can do that in one go and by creating a number of requirements.txt files as needed for the extras or dev dependencies. Here's a good starting documentation on how our pyproject.toml should look like: https://setuptools.pypa.io/en/latest/userguide/quickstart.html

Possible Alternatives

N/A

@idanov idanov added the Issue: Feature Request New feature or improvement to existing feature label Dec 22, 2022
@merelcht merelcht added Component: DevSetup Issue/PR that addresses technical setup of the project repository and removed Issue: Feature Request New feature or improvement to existing feature labels Jan 3, 2023
@merelcht merelcht added this to the Improve DevOps and DevSetup milestone Jan 10, 2023
@merelcht
Copy link
Member

We might be able to use this: https://github.com/asottile/setup-py-upgrade

@astrojuanlu
Copy link
Member

Related: I spotted this while running kedro package on the spaceflights tutorial:

/Users/juan_cano/.micromamba/envs/kedro310/lib/python3.10/site-packages/setuptools/command/install.py:34: SetuptoolsDeprecationWarning: setup.py install is deprecated. Use build and pip and other standards-based tools.

We should get rid of setup.py everywhere.

@sbrugman
Copy link
Contributor

sbrugman commented Jan 31, 2023

We can do that in one go and by creating a number of requirements.txt files as needed for the extras or dev dependencies.

That works, but the idiomatic way to achieve this effect with pyproject.toml is to have an optional-dependencies section, obviating the need for separate files.

For hierarchical setting (e.g. different dependencies for distinct pipelines), one can have pyproject.toml files to extend/override in these subfolders.

@astrojuanlu
Copy link
Member

astrojuanlu commented Mar 26, 2023

Notice that this doesn't have to be done in one go: setuptools supports having pyproject.toml for static stuff, and setup.py or external files for dynamic stuff.

For example, pyproject.toml can refer to an external file for dependency/requirements.txt https://setuptools.pypa.io/en/latest/userguide/pyproject_config.html#dynamic-metadata so even if ideally we would be migrating it to be in pyproject.toml directly, it doesn't have to be done straight away, especially because that file is used in behave:

@given("I have updated kedro requirements")
def update_kedro_req(context: behave.runner.Context):
"""Replace kedro as a standalone requirement with a line
that includes all of kedro's dependencies (-r kedro/requirements.txt)
"""
reqs_path = context.root_project_dir / "src" / "requirements.txt"
kedro_reqs = f"-r {context.requirements_path.as_posix()}"

The part that can't be avoided for now then is extras_require, as explained above.

Hence a first step towards having mostly static metadata is fairly straightforward:

  • Remove test_requires (it's deprecated)
  • Replace python setup.py in Makefile by the appropriate command (pip install build && python -m build)
  • Move everything except extras_require from setup.py to pyproject.toml (no more need to dynamically read README or version file, setuptools takes care of it)

astrojuanlu added a commit that referenced this issue Mar 28, 2023
See gh-2152.

Signed-off-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>
astrojuanlu added a commit that referenced this issue Apr 3, 2023
See gh-2152.

Signed-off-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>
astrojuanlu added a commit that referenced this issue Apr 3, 2023
* Remove deprecated tests_require

Signed-off-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>

* Migrate most project metadata to static pyproject.toml

See gh-2152.

Signed-off-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>

* Replace direct setup.py invokations by standard command

Signed-off-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>

---------

Signed-off-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>
MaximeSteinmetz pushed a commit to MaximeSteinmetz/kedro that referenced this issue Apr 3, 2023
)

* Remove deprecated tests_require

Signed-off-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>

* Migrate most project metadata to static pyproject.toml

See kedro-orggh-2152.

Signed-off-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>

* Replace direct setup.py invokations by standard command

Signed-off-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>

---------

Signed-off-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>
Signed-off-by: MaximeSteinmetz <maxime.steinmetz@gadz.org>
@astrojuanlu
Copy link
Member

Even though gh-2475 didn't technically obliterate setup.py, now that most of the project metadata is in pyproject.toml maybe we could just keep setup.py around for our extras mangling and close this issue. What do others think?

@merelcht
Copy link
Member

@astrojuanlu It looks like all extra requirements are dataset related, so those will be removed anyway when datasets are removed from Kedro core in 0.19.0. So I'm guessing there isn't really anything we can do until then?

@astrojuanlu
Copy link
Member

@merelcht good catch, yeah I agree!

@merelcht
Copy link
Member

We have #2127 for that, so I think we can indeed close this issue as done 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: DevSetup Issue/PR that addresses technical setup of the project repository
Projects
Archived in project
Development

No branches or pull requests

4 participants