-
Notifications
You must be signed in to change notification settings - Fork 224
TOPIC: Parsing PDFs table contents for RAG supporting notebook #341
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
Conversation
updated readme.md to include example of fetch surrounding chunks
Found 1 changed notebook. Review the changes at https://app.gitnotebooks.com/elastic/elasticsearch-labs/pull/341 |
updated file names
removed colab link to check if that is why build is faling.
adding pdf to branch
per instructions here: https://github.com/elastic/elasticsearch-labs/blob/main/CONTRIBUTING.md#automated-testing I fixed the notebook and the local test passed. ```*[sunman][~/Documents/GitHub/elasticsearch-labs]$ bin/nbtest supporting-blog-content/alternative-approach-for-parsing-pdfs-in-rag/alternative-approach-for-parsing-pdfs-in-rag.ipynb Running supporting-blog-content/alternative-approach-for-parsing-pdfs-in-rag/alternative-approach-for-parsing-pdfs-in-rag.ipynb... OK ```
created readme.md
LGTM. @justincastilla do you mind giving this a once over for me? |
On it! |
supporting-blog-content/alternative-approach-for-parsing-pdfs-in-rag/README.md
Show resolved
Hide resolved
supporting-blog-content/alternative-approach-for-parsing-pdfs-in-rag/README.md
Show resolved
Hide resolved
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.
Just a couple usability things I thought of that might make running this notebook more efficient for the user.
Co-authored-by: Justin Castilla <[email protected]>
added suggestions based on PR comments/review.
Added comments per slack discussion: https://elastic.slack.com/archives/C059SNT4X0R/p1729261191648229
@carlyrichmond can this be merged and closed? |
@carlyrichmond - Merging is blocked. All checks have passed. |
@sunilemanjee I see outstanding changes requested from @justincastilla. Have these been addressed? |
…nt Retrieval notebook First commit of Unifying Elastic Vector Database and LLMs for Intelligent Retrieval notebook. Passed local nbtest
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.
Approved!
This notebook is supporting article: https://github.com/elastic/search-labs-elastic-co/pull/1270