Migrate from '...requirements.txt' to 'pyproject.toml'

[MehdiNV]

Description

Draft PR, TBA

Resolves #1527

What's missing for the Draft PR?

• Making sure that setup.py has a 'package_data' parameter value passed into it

TL:DR

TBC

Development notes

TBC

QA notes

TBC

What's next?

TBC

[ravi-kumar-pilla]

unable to read the README file as it is in a different directory. We need to consider following the directory structure of other kedro-plugins to be consistent since kedro-viz is also a plugin.

@rashidakanchwala @noklam

Our directory structure - kedro-viz/package/kedro_viz
Other plugins have (example) - kedro-telemetry/kedro_telemetry .

Is it better we restructure our directory structure to kedro-viz/kedro_viz ?

[astrojuanlu]

It's tricky in your case because there's a separation between the backend and the frontend 🤔 ../README.md doesn't work?

[ravi-kumar-pilla]

yes it throws an error saying toml cannot access any files outside the directory it is in

[astrojuanlu]

you can use setup.py for reading the README.md, then setuptools will merge whatever is on setup.py with pyproject.toml. see what kedro-datasets does for example

[ravi-kumar-pilla]

@MehdiNV I tested removing the package_data and also deleted the setup.py, setup.cfg completely. I was able to install editable package via pip install -e package and also was able to run kedro viz. I am not sure where the jsbuild is used tbh.

@astrojuanlu - is the jsbuild used during publishing the package ? We execute the command make package and the below script runs -

package:
	find . -regex ".*/__pycache__" -exec rm -rf {} +
	find . -regex ".*\.egg-info" -exec rm -rf {} +
	test -f package/kedro_viz/html/index.html || (echo "Built npm package not found; packaging process cancelled."; exit 1)
	cd package && python setup.py clean --all
	cd package && python setup.py sdist bdist_wheel

@MehdiNV we might need to change these steps as well if we get rid of setup.py completely

@astrojuanlu - how can we test if the migration is success, apart from installing the package locally ? We also need to test the make package command works and the package includes everything as before ?

Thank you

[MehdiNV]

@ravi-kumar-pilla I think setup.py is only needed as a fallback for users who cannot utilise pyproject.toml for one reason or another (see stackoverflow post on it), what do you think about keeping it as a plan B? Or perhaps it's not necessary as it's very unlikely a user will have incompatability w/ .toml?

Overall from reading it, it seems that setup.py and others are just complementary parts to .toml - if toml setup fails, then setup.py acts as a fallback in its place for those who just have incompatible systems.

[ravi-kumar-pilla]

I think we can migrate the requirements.txt, test_requirements.txt, setup.cfg and part of setup.py (excluding the readme and package data) to pyproject.toml. I want to discover more on this and see how can we test the setup locally and how it affects our release process of make package I mentioned above. Will work on this today ! I feel we can have both pyproject.toml and setup.py similar to the kedro team

[astrojuanlu]

I think the way to test if the migration is successful is producing the final .tar.gz and .whl files that go to https://pypi.org/project/kedro-viz/#files, with and without this PR, and making sure that the relevant files are identical

[ravi-kumar-pilla]

Hi @astrojuanlu ,

Thank you for the input. I tested them and they seem to be the same. I have a question on installing dependencies.

Previously when we used to install dependencies of kedro-viz via requirements.txt file, kedro-viz was not installed as a package. However, when installing the same dependencies via pyproject.toml, the list of installed dependencies contain kedro-viz.

I checked this for kedro-datasets and even kedro-datasets get installed as dependency when we install its dependencies from pyproject.toml. Is this normal ?

Thank you

[ravi-kumar-pilla]

Since we are not moving to pyproject.toml completely, I do not see why this migration is needed. For handling the caveat mentioned, I feel we should go by the option - Configure dependabot to use widen. and migrate to pyproject.toml when the ticket pypa/pip#11440 is merged. What do you think of this approach ? @astrojuanlu @tynandebold

[astrojuanlu]

I'm supportive of that 👍🏽

[MehdiNV]

^ This makes sense; the .toml file is now linked to a requirements.txt file containing the dependencies, however I've kept the test_requirements.txt file deleted (installing test dependencies is via .toml).

In my head, a user should have no need for the test-dependencies unless they need to use / install kedro-viz as well (in which case, they'd use the .toml fully anyway). Is this a suitable approach?

[ravi-kumar-pilla]

@MehdiNV we will not go ahead with this draft PR. Instead, we will use the option of Configuring dependabot to use widen. to handle - #1582 (comment)

[MehdiNV]

Ahhh makes sense; apologies, I hadn't reloaded the page before so hadn't seen the new comments in the last hour when I was replying. Will shift towards the widen option then - will close this PR in the meantime as it's redundant

[astrojuanlu]

If I may, migrating from setup.py to pyproject.toml, even if it's partially, is still worth it. But you're right that if you want to keep requirements.txt around to be able to install only the dependencies, it's not as useful.