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.

Sign up for GitHub

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

Jump to bottom

Add pdf2parquet transform #416

Merged
merged 35 commits into from
Jul 25, 2024
Merged

Add pdf2parquet transform #416

merged 35 commits into from
Jul 25, 2024

Conversation

dolfim-ibm
Copy link
Member

Why are these changes needed?

This PR adds a transform for converting PDF files to Markdown

dolfim-ibm
dolfim-ibm commented Jul 17, 2024
View reviewed changes
.make.defaults Outdated Show resolved Hide resolved
daw3rd
daw3rd requested changes Jul 17, 2024
View reviewed changes
.make.defaults Outdated Show resolved Hide resolved
transforms/universal/pdf2md/README.md Outdated Show resolved Hide resolved
transforms/universal/pdf2md/README.md Outdated Show resolved Hide resolved
transforms/universal/pdf2md/kfp_ray/noop_multiple_wf.py Outdated Show resolved Hide resolved
transforms/universal/pdf2md/kfp_ray/noop_multiple_wf.py Outdated Show resolved Hide resolved
transforms/universal/pdf2md/python/src/pdf2md_transform.py Outdated Show resolved Hide resolved
transforms/universal/pdf2md/python/src/pdf2md_transform_python.py Outdated Show resolved Hide resolved
transforms/universal/pdf2md/python/test/test_pdf2md.py Outdated Show resolved Hide resolved
transforms/universal/pdf2md/ray/README.md Outdated Show resolved Hide resolved
transforms/universal/pdf2md/ray/README.md Outdated Show resolved Hide resolved
blublinsky
blublinsky reviewed Jul 17, 2024
View reviewed changes
transforms/universal/pdf2md/kfp_ray/README.md Outdated Show resolved Hide resolved
transforms/universal/pdf2md/python/README.md Outdated Show resolved Hide resolved
transforms/universal/pdf2md/python/src/pdf2md_transform.py Outdated

shortname = "pdf2md"
cli_prefix = f"{shortname}_"
pdf2md_modelsdir_key = f"{shortname}_modelsdir"
Copy link
Collaborator

Choose a reason for hiding this comment

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

why?

transforms/universal/pdf2md/python/src/pdf2md_transform.py Outdated Show resolved Hide resolved
transforms/universal/pdf2md/python/src/pdf2md_transform.py Outdated Show resolved Hide resolved
transforms/universal/pdf2md/python/src/pdf2md_transform.py Outdated Show resolved Hide resolved
transforms/universal/pdf2md/python/src/pdf2md_transform.py Outdated Show resolved Hide resolved
transforms/universal/pdf2md/python/src/pdf2md_transform.py Outdated Show resolved Hide resolved
transforms/universal/pdf2md/python/src/pdf2md_transform.py Outdated Show resolved Hide resolved
transforms/universal/pdf2md/python/README.md Outdated

| Parameter | Default | Description |
|------------|----------|--------------|
| `--pdf2md_modelsdir` | `./artifacts` | The location where the models are located or downloaded to |
Copy link
Collaborator

Choose a reason for hiding this comment

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

These are input parameters, so they are fine

daw3rd
daw3rd requested changes Jul 18, 2024
View reviewed changes
Copy link
Member

@daw3rd daw3rd left a comment

Choose a reason for hiding this comment

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

Separately, we are you using markdown and not raw text in the content column?

transforms/universal/pdf2md/python/README.md Outdated

| Parameter | Default | Description |
|------------|----------|--------------|
| `--pdf2md_modelsdir` | `./artifacts` | The location where the models are located or downloaded to |
Copy link
Member

Choose a reason for hiding this comment

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

@blublinsky no, we separately have config parameters to the transform and CLI parameters. this is well-established pattern in other transforms. My comment stands.

transforms/universal/pdf2md/python/src/pdf2md_transform.py Outdated Show resolved Hide resolved
transforms/universal/pdf2md/python/src/pdf2md_transform.py Outdated Show resolved Hide resolved
@daw3rd daw3rd changed the title Add Docling pdf2md Add pdf2parquet transform Jul 18, 2024
daw3rd
daw3rd requested changes Jul 18, 2024
View reviewed changes
transforms/universal/pdf2md/python/README.md Outdated

| Parameter | Default | Description |
|------------|----------|--------------|
| `--pdf2md_modelsdir` | `./artifacts` | The location where the models are located or downloaded to |
Copy link
Member

Choose a reason for hiding this comment

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

No, we should add the non-CLI config params that do not include pdf2md_ prefix to the README, then the CLI --pdf2md* params can just refer to that documentation. The point here is that there is configuration of the transform that is independent of the CLI. the prefix is used in the CLI to help better distinguish these within the very large set of params that are presented in the cloud/kfp gui to run at scale.

transforms/universal/pdf2md/python/Dockerfile Outdated Show resolved Hide resolved
transforms/universal/pdf2md/python/Makefile Outdated Show resolved Hide resolved
transforms/universal/pdf2md/python/.dockerignore Outdated Show resolved Hide resolved
transforms/universal/pdf2md/python/.gitignore Outdated Show resolved Hide resolved
daw3rd
daw3rd requested changes Jul 22, 2024
View reviewed changes
transforms/universal/pdf2parquet/python/README.md Outdated Show resolved Hide resolved
transforms/universal/pdf2parquet/python/src/pdf2parquet_transform.py Outdated Show resolved Hide resolved
transforms/universal/pdf2parquet/python/src/pdf2parquet_transform.py Outdated Show resolved Hide resolved
transforms/universal/pdf2parquet/python/test/test_pdf2parquet.py Outdated Show resolved Hide resolved
transforms/universal/pdf2parquet/python/src/pdf2parquet_transform.py Outdated Show resolved Hide resolved
transforms/universal/pdf2parquet/python/test/test_pdf2parquet_python.py Outdated Show resolved Hide resolved
transforms/universal/pdf2parquet/ray/Dockerfile Outdated Show resolved Hide resolved
transforms/universal/pdf2parquet/python/Dockerfile Outdated Show resolved Hide resolved
transforms/universal/pdf2parquet/ray/src/pdf2parquet_transform_ray.py Outdated Show resolved Hide resolved
daw3rd
daw3rd requested changes Jul 23, 2024
View reviewed changes
transforms/universal/pdf2parquet/python/Dockerfile Outdated Show resolved Hide resolved
transforms/universal/pdf2parquet/ray/Dockerfile Outdated Show resolved Hide resolved
@dolfim-ibm dolfim-ibm force-pushed the docling-pdf2md branch 2 times, most recently from 7b0ca63 to 8c4d0bf Compare July 24, 2024 19:06
@Bytes-Explorer Bytes-Explorer mentioned this pull request Jul 25, 2024
Closed
@dolfim-ibm
copy noop as skeleton
a0d485e 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
add docling for converting pdf documents to md
f6bed21 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
add DOCKER_PLATFORM variable
81ee45e 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
fix Makefile, improve dockerfile and address feedback
f92010d 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
add ray, simplify CLI params parsing, use Pdf2Md as name
58aa5d8 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
fix dockerfile and noop
9031fdf 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
add /src in root dir and avoid double COPY
d5dcc08 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
add transform_class in TransformConfiguration
e938f88 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
transform_class arg for ray, simplify cli args, remove download model…
bca0aaa 
…s (done automatically), cleanup prints

Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
add kfp
b378ad1 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
fixed for tests
32a0f40 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
fix test_pdf2md.py input/output
134c48c 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
fix reference data
6a5a466 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
ignore verbose contents since we check its content hash
526484a 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
rename to pdf2parquet
a6a0950 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
update expected results to match container versions
2a386e9 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
temporary add another CI test to validate pdf2parquet independently
4d65403 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
add run_locally
52df94f 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
typo and expand README
9efae7d 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
raise final failures
4531cc3 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
add free up space for test-universal
7e1624a 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
keep jvm
ec2b469 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
remove temporary build in CI
172760d 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
add option for json output
6d27e73 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
add docs
7c5efd7 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
py 3.10 compatibility
0e12d6e 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
fix test results
e31318b 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
restore proper test
b07f46e 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
remove blacklisting of contents
916eb1b 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
apply changes as in IBM#440
8c57e1a 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
move pdf2parquet to language
c07d01e 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
add ocr extra
340a0cc 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
add more tests for do_ocr and pdf2parquet_do_table_structure
2fdbf83 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
add pdf2parquet to the README
4d9a985 
Signed-off-by: Michele Dolfi <[email protected]>
@dolfim-ibm
extend timeout for the language images
8e06993 
Signed-off-by: Michele Dolfi <[email protected]>
daw3rd
daw3rd approved these changes Jul 25, 2024
View reviewed changes
Copy link
Member

@daw3rd daw3rd left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@daw3rd daw3rd merged commit 8360e18 into IBM:dev Jul 25, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blublinsky blublinsky blublinsky left review comments

@daw3rd daw3rd daw3rd approved these changes

@Param-S Param-S Awaiting requested review from Param-S

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dolfim-ibm @daw3rd @blublinsky