kedro-org / kedro-viz

Visualise your Kedro data and machine-learning pipelines and track your experiments.
https://demo.kedro.org
Apache License 2.0
672 stars 110 forks source link

Fix packaging #1766

Closed astrojuanlu closed 2 months ago

astrojuanlu commented 7 months ago

Description

Make sdists self-contained, hence fix #1267

Development notes

Commits:

  1. cf738296 Remove direct invokations of setup.py, which have been deprecated for about 2 years https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html
  2. 44538613 Migrate to pyproject.toml using @MehdiNV work in #1584 as baseline with some updates, hence fix #1527
  3. 495a54b1 Migrate from setuptools to hatchling https://hatch.pypa.io/, same backend vizro uses (cc @antonymilne), as a preparation to eventually address #1611 and also fix #1267
  4. e22a6ba0 Move README.md to package/ and create root symlink to retain GitHub preview

QA notes

  1. Run make package in main and with this PR, save the resulting sdist and wheel in dist.old and dist respectively
  2. Unzip the resulting .tar.gz, and compare the PKG-INFO files of both sdists - there should be only lines out of order and other non consequential changes directories with main and with this PR.
  3. Compare the tree structure of both sdists - only metadata files should have changed.
--- old.tree    2024-02-27 00:41:59
+++ new.tree    2024-02-27 00:42:29
@@ -1,5 +1,6 @@
-dist.old/kedro-viz-7.1.0/
+dist/kedro_viz-7.1.0/
 ├── PKG-INFO
+├── README.md
 ├── kedro_viz
 │   ├── __init__.py
 │   ├── api
@@ -81,10 +82,7 @@
 │       ├── __init__.py
 │       ├── layers.py
 │       └── modular_pipelines.py
-├── setup.cfg
-├── setup.py
-└── tests
-    ├── test_import.py
-    └── test_server.py
+├── pyproject.toml
+└── requirements.txt

-19 directories, 69 files
+18 directories, 68 files
  1. Unzip both wheel files and compare the *.dist-info directories - there should be only lines out of order but the number of files in RECORD should be basically the same.
...
diff --color=auto -u dist.old/kedro_viz-7.1.0.dist-info/RECORD dist/kedro_viz-7.1.0.dist-info/RECORD
--- dist.old/kedro_viz-7.1.0.dist-info/RECORD   2024-02-26 23:37:50
+++ dist/kedro_viz-7.1.0.dist-info/RECORD       2020-02-02 00:00:00
@@ -62,8 +62,7 @@
 kedro_viz/services/__init__.py,sha256=-ZfXYz5XecClJZK-cjZsoRrCpQCThwc3TXvCyDs0Sek,186
 kedro_viz/services/layers.py,sha256=_JO8mbmA0ZzAMN2dcXVaDX89Gr_QtJDWtmDnUeX49gc,5263
 kedro_viz/services/modular_pipelines.py,sha256=Tx3ekpwjt981jmljezxgWlOVaWdxgVADKCY1QyIUC4Q,4064
-kedro_viz-7.1.0.dist-info/METADATA,sha256=j_RijhLdEdn3vEgHlpCUDyCGHq6u4Ho8zkVJ9VT_xwE,11619
-kedro_viz-7.1.0.dist-info/WHEEL,sha256=oiQVh_5PnQM0E3gPdiz09WCNmwiHDMaGer_elqB3coM,92
+kedro_viz-7.1.0.dist-info/METADATA,sha256=tkqndzIrnu7hy0Pnj760FEOCApMYIBI9n0e2gxoo6vg,11759
+kedro_viz-7.1.0.dist-info/WHEEL,sha256=TJPnKdtrSue7xZ_AVGkp9YXcvDrobsjBds1du3Nx6dc,87
 kedro_viz-7.1.0.dist-info/entry_points.txt,sha256=yCRpAmWDqux5fspKHZ03tRjHkmIjs8ypb1m3XuvWNMI,228
-kedro_viz-7.1.0.dist-info/top_level.txt,sha256=gWY-UrKtMq-3SUTv5Zww2hc6_ldAln47aYqCihTOyew,10
 kedro_viz-7.1.0.dist-info/RECORD,,
  1. Verify that pip install ./dist.old/...tar.gz doesn't work (hence #1267) and pip install ./dist/...tar.gz fixes the issue
  2. Launch kedro viz run on a test project after installing the new sdist, verify that everything works as expected (the HTML files are included)

Checklist

astrojuanlu commented 7 months ago
package/kedro_viz/data_access/repositories/graph.py:46:8: R1737: Use 'yield from' directly instead of yielding each element one by one (use-yield-from)

linting failures unrelated

astrojuanlu commented 7 months ago

The README.md hack didn't get me very far though, now the e2e tests are failing. Exhausted a couple of avenues with Hatch and gave up for the day, asked a upstream https://github.com/pypa/hatch/discussions/1286

rashidakanchwala commented 7 months ago

This is amazing!!!! -- thanks @astrojuanlu ! liking the nerd-snipe you.

antonymilne commented 7 months ago

Amazing PR! This is a HUGE improvement.

Just two things I'm curious about:

  1. What is the motivation for "https://github.com/kedro-org/kedro-viz/commit/e22a6ba0df118d2941ec26770cc1f8d59704db68 Move README.md to package/ and create root symlink to retain GitHub preview"?
  2. What further steps would be needed to make pip install git+... on kedro-viz work?
astrojuanlu commented 7 months ago
  1. setup.py had a open("../README.md"), that's not possible with static pyproject.toml (neither with setuptools nor with hatchling). I asked upstream https://github.com/pypa/hatch/discussions/1286 so far no response
  2. That's the topic of https://github.com/kedro-org/kedro-viz/issues/1611 and will require a custom hook that builds the assets upon installation, but that's for a separate PR 🙃
astrojuanlu commented 2 months ago

Thanks @ravi-kumar-pilla ! I’ll double check the final state after you solved the conflicts

ravi-kumar-pilla commented 2 months ago

This looks good to me 👍🏼 Beware of the moving README.md as it will conflict with #1745 @jitu5 fyi