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

feat: integrate got-ocr2.0 as image reader #355

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

phv2312
Copy link
Contributor

@phv2312 phv2312 commented Oct 2, 2024

Description

  • Integrate the got-ocr2.0 OCR as image reader
  • New extension manager for easily switch between different supported loaders
  • Also, thanks @cin-jimmy for his suggestion on github stale (issue)

Type of change

  • New features (non-breaking change).
  • Bug fix (non-breaking change).
  • Breaking change (fix or feature that would cause existing functionality not to work as expected).

Checklist

  • I have performed a self-review of my code.
  • I have added thorough tests if it is a core feature.
  • There is a reference to the original bug report and related work.
  • I have commented on my code, particularly in hard-to-understand areas.
  • The feature is well documented.

@phv2312 phv2312 changed the title Feat/integrate 3rd feat: integrate 3rd Oct 2, 2024
@phv2312 phv2312 changed the title feat: integrate 3rd feat: integrate got-ocr2.0 Oct 2, 2024
@phv2312 phv2312 changed the title feat: integrate got-ocr2.0 feat: integrate got-ocr2.0 as image reader Oct 2, 2024
@phv2312 phv2312 requested a review from taprosoft October 2, 2024 15:43
@cin-niko
Copy link
Contributor

cin-niko commented Oct 4, 2024

@phv2312, can you add a docker-compose file (allow choose the docker image for OCR service)? I think it will help people test more easily.

@phv2312 phv2312 force-pushed the feat/integrate_3rd branch from 25904f0 to de703a3 Compare October 25, 2024 07:55
@Cinnamon Cinnamon deleted a comment from ngduyanhece Oct 25, 2024
@phv2312
Copy link
Contributor Author

phv2312 commented Oct 26, 2024

Hi @taprosoft @cin-niko. Sorry for no update for long time. Can you help to review this PR again

@phv2312 phv2312 requested a review from taprosoft October 27, 2024 05:27
docker-compose.dev.yml Outdated Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
integration/got-ocr2.md Show resolved Hide resolved
libs/kotaemon/kotaemon/indices/ingests/extensions.py Outdated Show resolved Hide resolved
libs/kotaemon/kotaemon/indices/ingests/extensions.py Outdated Show resolved Hide resolved
libs/kotaemon/kotaemon/indices/ingests/extensions.py Outdated Show resolved Hide resolved
@phv2312
Copy link
Contributor Author

phv2312 commented Dec 15, 2024

Hi @cin-niko and @taprosoft . I have updated according to niko's comments and rebased from the latest master already.
Can you help to check this PR again ?

@cin-niko
Copy link
Contributor

@phv2312 Overall is good. But it seems that setting the loader for extensions feature doesn't work.
For example:

  • Set pdf loader in Settings -> Retrieval Settings -> File loader: Work
  • Set pdf loader in Settings -> Loader settings -> Loader .pdf: Doesn't work

@taprosoft
Copy link
Collaborator

@phv2312 sorry for the late comment. Overall the logic is fine but the current settings UI is a bit cluttered. I will push a small change to improve this prior to merging.

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