-
Notifications
You must be signed in to change notification settings - Fork 15.7k
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
[Community]: Image Extraction Fixed for PDFPlumberParser
#28491
base: master
Are you sure you want to change the base?
[Community]: Image Extraction Fixed for PDFPlumberParser
#28491
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
2 similar comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like @c-xlenz had some feedback worth addressing: #28480 (comment)
libs/community/pyproject.toml
Outdated
@@ -41,6 +41,7 @@ dataclasses-json = ">= 0.5.7, < 0.7" | |||
pydantic-settings = "^2.4.0" | |||
langsmith = "^0.1.125" | |||
httpx-sse = "^0.4.0" | |||
pillow = "^11.0.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we shouldn't be adding any new required dependencies — is there any other way to fix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@baskaryan The way I see it no.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@baskaryan let me know what I can do then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this fix, I need pillow otherwise it won't work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@keenborder786, optional dependencies can be OK, but not required ones
Also please get in the habit of including tests with PRs. We want to avoid a situation where a bug fix, is introducing new bug fixes (e.g., #28480 (comment))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eyurtsev okay.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eyurtsev @baskaryan I have removed pillow
as a required dependency. Please check now.
PDFPlumberParser