Build transforms wheel #493

touma-I · 2024-08-09T14:31:08Z

Why are these changes needed?

Allow users to do pip install of all the transforms without having to clone the project.
This process clones the project behind the scene and installs the various transforms.
Also account for the transforms that do not run on MacOS

Related issue number (if any).

Signed-off-by: Maroun Touma <[email protected]>

daw3rd

Can we get a

Makefile in each new directory and support the build, test, publish, clean and set-versions targets.
A readme in each directory.
Extra credit for a pypi-specific readme

transforms/packaging/python/pyproject.toml

Signed-off-by: Maroun Touma <[email protected]>

blublinsky · 2024-08-13T07:38:04Z

transforms/packaging/python/Makefile

+TRANSFORMS_NAMES = code/code_quality \
+    	code/code2parquet \
+    	code/header_cleanser \
+		code/code_quality \


code2parquet, malware

malware transforms need some additional work to be included as a pip install and was intentionally excluded from this initial release... It seem to require a docker container and does not fit in the current use case for running these transforms in a notebook.

blublinsky · 2024-08-13T07:41:17Z

transforms/packaging/python/README.md

+    * [code2parquet](https://github.com/IBM/data-prep-kit/blob/dev/transforms/code/code2parquet/python/README.md)
+    * header_cleanser (Not available on MacOS)
+    * code_quality
+    * proglang_select


malware transforms need some additional work to be included as a pip install and was intentionally excluded from this initial release... It seem to require a docker container and does not fit in the current use case for running these transforms in a notebook.

blublinsky · 2024-08-13T07:43:09Z

transforms/packaging/ray/README.md

+	* proglang_select
+	* header_cleanser (Not available on MacOS)
+	* code_quality
+	* repo_level_ordering


malware transforms need some additional work to be included as a pip install and was intentionally excluded from this initial release... It seem to require a docker container and does not fit in the current use case for running these transforms in a notebook.

blublinsky · 2024-08-13T07:44:05Z

transforms/packaging/ray/README.md

+	* profiler
+	* doc_id
+	* filter
+	* resize


@blublinsky what is meant by this comment?

missing transform

Signed-off-by: Maroun Touma <[email protected]>

daw3rd

You also need a recursive Makefile in transforms/packaging directory.

transforms/packaging/python/Makefile

transforms/packaging/python/pyproject.toml

daw3rd · 2024-08-27T22:12:30Z

transforms/packaging/ray/pyproject.toml

@@ -0,0 +1,53 @@
+[project]
+name = "data_prep_toolkit_transforms_ray"
+version = "0.2.1.dev1"


Signed-off-by: Maroun Touma <[email protected]>

data-processing-lib/ray/pyproject.toml

Signed-off-by: Maroun Touma <[email protected]>

daw3rd · 2024-09-13T20:05:38Z

@touma-I I think you will also need to update the .github/workflows/test.yml to add a job for this packaging directory.

Signed-off-by: Maroun Touma <[email protected]>

Signed-off-by: David Wood <[email protected]>

Signed-off-by: Maroun Touma <[email protected]>

… into build-transforms-wheel

Signed-off-by: Maroun Touma <[email protected]>

daw3rd

I think you may need to modify scripts/check-transform-workflows.sh to not include the packaging directory.

.github/workflows/test-packaging-python.yml

shivdeep-singh-ibm

lgtm

roytman · 2024-09-24T10:37:04Z

.make.defaults

@@ -480,7 +480,8 @@ endif
 	if [ -e requirements.txt ]; then				\
 		echo Installing requirements from requirements.txt;	\
 		pip install $(PIP_INSTALL_EXTRA_ARGS) $$extra_url -r requirements.txt;		\
-	elif [ -e pyproject.toml ]; then				\
+	fi;	\
+	if [ -e pyproject.toml ]; then				\


so you want to allow dependencies installation from both requirements.txt and pyproject.toml. It can confuse.
I'd add a WARNING message if both files exist. And specify in the message the installation order: first from requirements.txtand after that frompyproject.toml`

Thanks @roytman . The situation we ran into is the pyproject.toml for the single package is using requirements.txt that was compiled from the different dependencies listed by all transforms. I will also be changing all the transforms pyproject.toml to use requirements.txt to list their dependencies. But agree, I need to watch this one closely.

roytman · 2024-09-24T10:41:55Z

.make.defaults

@@ -587,6 +588,18 @@ MINIO_ADMIN_PWD= localminiosecretkey
 		> tt.toml;				\
 	    mv tt.toml pyproject.toml;			\
 	 fi
+	@if [ -e requirements.txt ]; then					\
+	    cat requirements.txt | sed 									\
+		-e 's/data-prep-toolkit-ray\([=><~][=]\).*/data-prep-toolkit-ray\1$(DPK_LIB_VERSION)/' 	\


can all these sed operations be replaced by a single macro? the lines 582-587 are almost identical to lines 593-599.

Yes. Almost but not exactly. But agree, I will take a deeper look in a followup PR.

roytman · 2024-09-24T10:44:18Z

examples/notebooks/rag/requirements.txt

@@ -1,6 +1,6 @@
 ## Data prep kit
-data-prep-toolkit-transforms==0.2.1.dev1
-data-prep-toolkit-transforms-ray==0.2.1.dev1
+#data-prep-toolkit-transforms==0.2.1.dev1


I see that these are comments, but should they be with 0.2.1.dev3 ?

I am not sure this notebook will still work with dev3. There were changes to the transforms that broke the notebook. I think Sunjee has a new release he will be checking in soon.

roytman · 2024-09-24T10:47:16Z

transforms/packaging/.gitignore

@@ -0,0 +1,5 @@
+**/src


should we combine it (except src) with the .gitignore in the root directory?

That would work to. I think there may be situations in the future where we want to. add the wheel for specific transforms to git so folks can download it from git. This is still under discussion but yes, once we have consensus, we should move the gitignore to higher level as appropriate.

roytman · 2024-09-24T10:52:57Z

transforms/packaging/README.md

+
+
+````
+cd package-release/transforms/packaging


I think users are in package-release, so the command should be cd transforms/packaging

done! Thanks

roytman · 2024-09-24T10:54:22Z

transforms/packaging/README.md

+This procedure will run all the UT for each individual transforms using a single package configuration:
+
+````
+cd package-release/transforms/packaging


as above, or write something like, if you are not in the package-release/transforms/packaging, cd into it

Done! Thanks

roytman · 2024-09-24T10:54:41Z

transforms/packaging/README.md

+This procedure will buid two wheels: one for the python transforms and one for the ray transforms.
+
+````
+cd package-release/transforms/packaging


Done! Thanks

roytman · 2024-09-24T10:58:42Z

transforms/packaging/python/README.md

+	* [resize](https://github.com/IBM/data-prep-kit/blob/dev/transforms/code/resize/python/README.md)
+	* [tokenization](https://github.com/IBM/data-prep-kit/blob/dev/transforms/tokenization/doc_chunk/python/README.md)
+	* [doc_id](https://github.com/IBM/data-prep-kit/blob/dev/transforms/code/doc_id/python/README.md)
+


we cannot keep this list up to date, so it might be worth adding something like the list includes some of the transformers but doesn't all of them.

Done! Thanks

Signed-off-by: Maroun Touma <[email protected]>

touma-I · 2024-09-24T11:49:25Z

I think you may need to modify scripts/check-transform-workflows.sh to not include the packaging directory.

daw3rd · 2024-09-24T14:58:33Z

.github/workflows/test-packaging-python.yml

+            - "*"
+        paths:
+            - "transforms/packaging/python/**"
+            - "data-processing-lib/**"


If you don't include the transforms, you should probable not include the core library either.

i pushed a fix for this

daw3rd · 2024-09-24T14:58:59Z

.github/workflows/test-packaging-ray.yml

+            - "*"
+        paths:
+            - "transforms/packaging/ray/**"
+            - "data-processing-lib/**"


same, library not needed.

i pushed a fix for this.

daw3rd · 2024-09-24T15:03:58Z

transforms/packaging/Makefile

@@ -0,0 +1,60 @@
+REPOROOT=../../
+# Use make help, to see the available rules
+include ../../.make.defaults


should include $(REPOROOT)/.make.defaults and in general use REPOROOT

daw3rd · 2024-09-24T15:05:59Z

transforms/packaging/python/Makefile

+#  code/repo_level_ordering # Missing implementation
+
+
+TRANSFORMS_NAMES = code/code_quality \


We should build this list automatically by looking for the python directories in transforms//

daw3rd · 2024-09-24T15:10:25Z

transforms/packaging/ray/requirements.txt

i guess this file being created by hand? is the plan to eventually automate this file.

Let's address those in a separate PR. Thanks

Signed-off-by: David Wood <[email protected]>

daw3rd

i fixed workflow paths. others are minor comments.

touma-I added 2 commits August 8, 2024 18:59

pytest for most but faild for pdf2parquet and resize

ddedccc

Signed-off-by: Maroun Touma <[email protected]>

header_cleanser available for non Darwin installation

134c2c2

Signed-off-by: Maroun Touma <[email protected]>

daw3rd reviewed Aug 9, 2024

View reviewed changes

transforms/packaging/python/pyproject.toml Outdated Show resolved Hide resolved

touma-I added 3 commits August 9, 2024 23:56

Added make file with unit tests for all transforms

e227b9d

Signed-off-by: Maroun Touma <[email protected]>

Build and deploy pypi package using makefile

d155a24

Signed-off-by: Maroun Touma <[email protected]>

remove pandas and keep pandas=2.2.2

fe0d380

Signed-off-by: Maroun Touma <[email protected]>

blublinsky reviewed Aug 13, 2024

View reviewed changes

touma-I added 2 commits August 21, 2024 10:11

Merge branch 'dev' into build-transforms-wheel

b1e2707

added PII and HTML2Parquet

d96477a

Signed-off-by: Maroun Touma <[email protected]>

daw3rd requested changes Aug 27, 2024

View reviewed changes

touma-I added 11 commits August 28, 2024 04:29

Merge branch 'dev' into build-transforms-wheel

3c57682

Update with latest available transforms

35c7e60

Signed-off-by: Maroun Touma <[email protected]>

restructure things to be able to test and build independently

d66df27

Signed-off-by: Maroun Touma <[email protected]>

publish dev2 for python

a4f7e0a

Signed-off-by: Maroun Touma <[email protected]>

finish testing and publish dev2 python and ray packages:

0af03cc

Signed-off-by: Maroun Touma <[email protected]>

merge with dev

65f4ac4

Signed-off-by: Maroun Touma <[email protected]>

try different dependencies in attempt to resolve conflicts

703ebe0

Signed-off-by: Maroun Touma <[email protected]>

added pii redactor

d54708a

Signed-off-by: Maroun Touma <[email protected]>

Merge branch 'dev' into build-transforms-wheel

e1309e5

updated with latest release for pdf2parquet

109ea29

Signed-off-by: Maroun Touma <[email protected]>

fixes for dev3 release

a874358

Signed-off-by: Maroun Touma <[email protected]>

touma-I requested a review from daw3rd September 11, 2024 20:08

daw3rd requested changes Sep 12, 2024

View reviewed changes

data-processing-lib/ray/pyproject.toml Show resolved Hide resolved

simplify test-src by using exiting targets

440975d

Signed-off-by: Maroun Touma <[email protected]>

touma-I added 4 commits September 23, 2024 10:39

Merge branch 'dev' into build-transforms-wheel

10c9159

renamed requirement files

db60963

Signed-off-by: Maroun Touma <[email protected]>

use - in transform library name

9bb36c5

Signed-off-by: Maroun Touma <[email protected]>

update requirements.txt files as appropriate when setting versions

ee63628

Signed-off-by: Maroun Touma <[email protected]>

touma-I and others added 8 commits September 23, 2024 15:19

Added readme file to test, build and publish package to pypi

346b82e

Signed-off-by: Maroun Touma <[email protected]>

fix typos and removed double quotes

071836e

Signed-off-by: Maroun Touma <[email protected]>

Apply version update to all transforms

07b827f

Signed-off-by: Maroun Touma <[email protected]>

generate workflow for packaging folder

0369842

Signed-off-by: Maroun Touma <[email protected]>

fix ededup dummy version

aa297e0

Signed-off-by: David Wood <[email protected]>

fix filter/spark dummy version

32578d5

Signed-off-by: David Wood <[email protected]>

updated requirements based on latest release for docling

3b52ecf

Signed-off-by: Maroun Touma <[email protected]>

Merge branch 'build-transforms-wheel' of github.com:IBM/data-prep-kit…

eb499de

… into build-transforms-wheel

touma-I requested a review from daw3rd September 23, 2024 18:57

touma-I marked this pull request as ready for review September 23, 2024 18:58

touma-I requested a review from shivdeep-singh-ibm September 23, 2024 19:06

touma-I added 2 commits September 23, 2024 21:54

fix missing steps

8d69b71

Signed-off-by: Maroun Touma <[email protected]>

-sUpdate makefile to build and publish wheels

d27a1c2

daw3rd requested changes Sep 23, 2024

View reviewed changes

.github/workflows/test-packaging-python.yml Outdated Show resolved Hide resolved

touma-I requested a review from roytman September 24, 2024 09:06

shivdeep-singh-ibm reviewed Sep 24, 2024

View reviewed changes

roytman reviewed Sep 24, 2024

View reviewed changes

update based on reviewers comments

e155d2c

Signed-off-by: Maroun Touma <[email protected]>

touma-I closed this Sep 24, 2024

touma-I reopened this Sep 24, 2024

daw3rd previously requested changes Sep 24, 2024

View reviewed changes

fix packaging test workflow paths

33b8853

Signed-off-by: David Wood <[email protected]>

daw3rd approved these changes Sep 24, 2024

View reviewed changes

shivdeep-singh-ibm approved these changes Sep 24, 2024

View reviewed changes

touma-I merged commit f5d9680 into dev Sep 24, 2024
77 checks passed

touma-I deleted the build-transforms-wheel branch September 24, 2024 16:54

		# code/repo_level_ordering # Missing implementation


		TRANSFORMS_NAMES = code/code_quality \



		````
		cd package-release/transforms/packaging

Build transforms wheel #493

Build transforms wheel #493

Conversation

touma-I commented Aug 9, 2024

Why are these changes needed?

Related issue number (if any).

daw3rd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daw3rd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daw3rd commented Sep 13, 2024

daw3rd left a comment

Choose a reason for hiding this comment

shivdeep-singh-ibm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

touma-I commented Sep 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daw3rd left a comment • edited Loading

Choose a reason for hiding this comment

daw3rd left a comment •

edited

Loading