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

refactor tokenization transform as its own named dpk_ module #886

Merged
merged 10 commits into from
Dec 20, 2024

Conversation

matouma
Copy link
Contributor

@matouma matouma commented Dec 17, 2024

Why are these changes needed?

First pass at restructuring the transform built as its own module (e.g. dpk_tokenization) with a ray submodule (dpk_tokenization.ray ).

Removed python and ray folders and refactor Dockerfile.python and Dockerfile.ray
remove pyproject.toml and Makefiles
move python code under dpk_tokenization
move ray code under dpk_tokenization/ray
change import statement to include module name
replace recursive Makefile and use targets from .make.cicd.targets
adapt kfp_ray/Makefile and other make target

Related issue number (if any).

#774

Signed-off-by: Maroun Touma <[email protected]>
@matouma matouma marked this pull request as ready for review December 17, 2024 22:20
@touma-I touma-I changed the title merge with dev refactor tokenization transform as its own named dpk_ module Dec 17, 2024
Signed-off-by: Maroun Touma <[email protected]>
Signed-off-by: Maroun Touma <[email protected]>
Signed-off-by: Maroun Touma <[email protected]>
Signed-off-by: Maroun Touma <[email protected]>
Signed-off-by: Maroun Touma <[email protected]>
Signed-off-by: Maroun Touma <[email protected]>
Copy link
Member

@shahrokhDaijavad shahrokhDaijavad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

README file looks good to me.

Copy link
Collaborator

@dangxuanhong dangxuanhong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The re-structuring looks good to me! Also, using >= is better than using == for specifying required version in most requirements.txt. Thanks

@touma-I touma-I merged commit 62e3f97 into IBM:dev Dec 20, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants