Consolidate all requirements #2597

astrojuanlu · 2023-05-22T08:28:02Z

Description

Development notes

Moved dependency/requirements.txt to pyproject.toml, removed relevant code from setup.py
Moved test_requirements.txt to setup.py (where all the optional dependencies live at the moment)
Removed all references to test_requirements.txt and dependency/requirements.txt in CI
Adjusted various dependencies

Notice that I had to partially revert https://github.com/quantumblacklabs/private-kedro/pull/365 (private link). I did a quick test and the change seemed innocuous, but we should make sure that there are no problems when making a release (see original rationale https://github.com/quantumblacklabs/private-kedro/pull/352#issuecomment-564948371 also private)

Tested on Gitpod (Linux) on 3.7, 3.8, 3.9, and 3.10 ✔️ pip check always successful after pip install .[all,test] (no conflicting requirements).

Checklist

Read the contributing guidelines
Opened this PR as a 'Draft Pull Request' if it is work-in-progress
Updated the documentation to reflect the code changes
Added a description of this change in the RELEASE.md file
Added tests to cover my changes

sbrugman · 2023-06-13T10:52:52Z

error in project_dummy setup command: 'install_requires' must be a string or list of strings containing valid project/version requirement specifiers; Expected package name at the start of dependency specifier

Might be solved by upgrading setuptools (otherwise the requirements syntax is probably invalid).

Great to see progress on moving to pyproject.toml. Would be amazing if Kedro, the first-party Kedro plugins and the starter templates set the right standard (unfortunately not there yet)!

astrojuanlu · 2023-06-13T11:47:55Z

Thanks for the encouragement @sbrugman! Indeed that's the idea, we want to move everything to modern packaging standards. This is the way 🚀

features/steps/cli_steps.py

astrojuanlu · 2023-07-12T14:24:41Z

🤷🏽 pylint-dev/pylint#8845

I'm not waiting for pylint to fix this though, will ignore this for good

astrojuanlu · 2023-07-13T07:23:38Z

I removed import sorting from pylint since we were using isort anyway. All tests passing 👍🏽

astrojuanlu · 2023-07-13T07:25:34Z

features/steps/cli_steps.py


    if reqs_path.is_file():
        old_reqs = reqs_path.read_text().splitlines()
        new_reqs = []
        for req in old_reqs:
-            if req.startswith("kedro"):
-                new_reqs.append(kedro_reqs)
+            if req.startswith("kedro") and Requirement(req).name.lower() == "kedro":


The logic before was to replace kedro with all kedro dependencies. In a way, something like pip install kedro --dependencies-only (which doesn't exist I believe).

Now I'm just removing kedro, because this is only for our tests and the behave environment has the development version of kedro preinstalled already.

Also I made one improvement: parsing the requirement name so we are only dropping kedro and not, say, kedro-datasets or kedro-airflow. This was incorrect before.

astrojuanlu · 2023-07-13T07:27:28Z

setup.py

@@ -139,10 +132,77 @@ def _collect_requirements(requires):
 }

 extras_require["all"] = _collect_requirements(extras_require)
+extras_require["test"] = [


These are more or less the same as kedro-datasets[all], but since we're removing datasets from framework soon, I just copy pasted test_requirements.txt here and didn't aim to substantially change what we're doing. Most of these can go away in 0.19.0.

merelcht

Thank you for making this change @astrojuanlu ⭐ I'm glad the requirements are cleaned up like this. I would add this change to the release notes as well.

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

kedro is preinstalled in the behave environment, so this should not be needed. In exchange, requirements.txt files can always be read by setuptools automatic parsing. See McK-Private/private-kedro#352 (comment) for motivation of the original logic. Signed-off-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>

Fix gh-2588. 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>

SajidAlamQB

Thank you @astrojuanlu!

* Empty Signed-off-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com> * Avoid adding problematic lines to requirements.txt kedro is preinstalled in the behave environment, so this should not be needed. In exchange, requirements.txt files can always be read by setuptools automatic parsing. See McK-Private/private-kedro#352 (comment) for motivation of the original logic. Signed-off-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com> * Consolidate all requirements Fix gh-2588. Signed-off-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com> * Fix dependencies Signed-off-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com> * Ignore import order in pylint in favour of isort Signed-off-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com> * Do not replace kedro plugins in e2e tests Signed-off-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com> * Fix dependabot config Signed-off-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com> * Add release notes 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: Nok <nok.lam.chan@quantumblack.com>

astrojuanlu mentioned this pull request May 22, 2023

Install Kedro, plugin, and test requirements in a single pip install command #2588

Closed

This comment was marked as resolved.

Sign in to view

astrojuanlu mentioned this pull request May 23, 2023

Move default template to static pyproject.toml #2569

Closed

5 tasks

This comment was marked as resolved.

Sign in to view

astrojuanlu mentioned this pull request May 29, 2023

micropkg packaging improvements #2614

Merged

5 tasks

astrojuanlu force-pushed the dev/fix-requirements branch 3 times, most recently from 4e7d2a8 to 42145d5 Compare July 10, 2023 23:32

astrojuanlu commented Jul 11, 2023

View reviewed changes

features/steps/cli_steps.py Outdated Show resolved Hide resolved

astrojuanlu force-pushed the dev/fix-requirements branch from 42145d5 to 3a4b52d Compare July 11, 2023 07:11

This comment was marked as off-topic.

Sign in to view

This comment was marked as resolved.

Sign in to view

astrojuanlu marked this pull request as ready for review July 11, 2023 07:20

astrojuanlu requested a review from merelcht as a code owner July 11, 2023 07:20

astrojuanlu requested review from SajidAlamQB, AhdraMeraliQB and ankatiyar July 11, 2023 07:21

This comment was marked as resolved.

Sign in to view

astrojuanlu force-pushed the dev/fix-requirements branch from 3a4b52d to dd04cb0 Compare July 12, 2023 12:46

astrojuanlu closed this Jul 12, 2023

astrojuanlu reopened this Jul 12, 2023

astrojuanlu force-pushed the dev/fix-requirements branch from 140f146 to dd04cb0 Compare July 12, 2023 12:53

astrojuanlu force-pushed the dev/fix-requirements branch 4 times, most recently from a024621 to 0bba85f Compare July 12, 2023 15:50

astrojuanlu commented Jul 13, 2023

View reviewed changes

astrojuanlu force-pushed the dev/fix-requirements branch from 0bba85f to 8cc2ca5 Compare July 14, 2023 10:40

merelcht approved these changes Jul 14, 2023

View reviewed changes

This comment was marked as resolved.

Sign in to view

astrojuanlu added 8 commits July 14, 2023 17:39

Empty

23da726

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

Consolidate all requirements

e790eb0

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

Fix dependencies

181659f

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

Ignore import order in pylint in favour of isort

4239c2c

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

Do not replace kedro plugins in e2e tests

e2cdd7a

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

Fix dependabot config

ceffb66

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

Add release notes

4104e68

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

astrojuanlu force-pushed the dev/fix-requirements branch from 8cc2ca5 to 4104e68 Compare July 14, 2023 15:46

datajoely approved these changes Jul 18, 2023

View reviewed changes

SajidAlamQB approved these changes Jul 18, 2023

View reviewed changes

astrojuanlu merged commit 4eb6d87 into main Jul 18, 2023
35 checks passed

astrojuanlu deleted the dev/fix-requirements branch July 18, 2023 09:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Consolidate all requirements #2597

Consolidate all requirements #2597

astrojuanlu commented May 22, 2023 •

edited

This comment was marked as resolved.

This comment was marked as resolved.

sbrugman commented Jun 13, 2023 •

edited

astrojuanlu commented Jun 13, 2023

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as resolved.

This comment was marked as resolved.

astrojuanlu commented Jul 12, 2023

astrojuanlu commented Jul 13, 2023

astrojuanlu Jul 13, 2023

astrojuanlu Jul 13, 2023

astrojuanlu Jul 13, 2023

merelcht left a comment

This comment was marked as resolved.

SajidAlamQB left a comment

Consolidate all requirements #2597

Consolidate all requirements #2597

Conversation

astrojuanlu commented May 22, 2023 • edited

Description

Development notes

Checklist

This comment was marked as resolved.

This comment was marked as resolved.

sbrugman commented Jun 13, 2023 • edited

astrojuanlu commented Jun 13, 2023

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as resolved.

This comment was marked as resolved.

astrojuanlu commented Jul 12, 2023

astrojuanlu commented Jul 13, 2023

astrojuanlu Jul 13, 2023

Choose a reason for hiding this comment

astrojuanlu Jul 13, 2023

Choose a reason for hiding this comment

astrojuanlu Jul 13, 2023

Choose a reason for hiding this comment

merelcht left a comment

Choose a reason for hiding this comment

This comment was marked as resolved.

SajidAlamQB left a comment

Choose a reason for hiding this comment

astrojuanlu commented May 22, 2023 •

edited

sbrugman commented Jun 13, 2023 •

edited