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

community: Microsoft Azure Document Intelligence updates #16932

Merged
merged 23 commits into from
Mar 27, 2024
Merged

community: Microsoft Azure Document Intelligence updates #16932

merged 23 commits into from
Mar 27, 2024

Conversation

ruoccofabrizio
Copy link
Contributor

  • Description: Update Azure Document Intelligence implementation by Microsoft team and RAG cookbook with Azure AI Search

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Feb 2, 2024
@vercel Vercel
Copy link

vercel bot commented Feb 2, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
langchain ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 27, 2024 5:48am

@dosubot dosubot bot added Ɑ: doc loader Related to document loader module (not documentation) 🤖:improvement Medium size change to existing code to handle new use-cases labels Feb 2, 2024
@ruoccofabrizio
Copy link
Contributor Author

@luzhang06 @toothache

baskaryan
baskaryan reviewed Feb 6, 2024
View reviewed changes
Copy link
Collaborator

@baskaryan baskaryan left a comment

Choose a reason for hiding this comment

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

thanks @ruoccofabrizio!! few comments

cookbook/rag_semantic_chunking.ipynb Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

could we add something to file name to make it clear it's using azure doc intelligence?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure. Updated the notebook file name.

libs/community/langchain_community/document_loaders/parsers/doc_intelligence.py Outdated
@@ -15,24 +18,40 @@ def __init__(
api_endpoint: str,
api_key: str,
api_version: Optional[str] = None,
api_model: str = "prebuilt-layout",
model_id: str = "prebuilt-layout",
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is breaking change, what's value of doing this?

Copy link
Contributor

Choose a reason for hiding this comment

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

@toothache

Copy link
Contributor

Choose a reason for hiding this comment

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

This is to align with the SDK argument names.

modelId: Unique document model name. Required.

Besides, I intentionally changed the parser class only, the loader is not changed. Will it also bring the compatibility issue?

Copy link
Contributor

Choose a reason for hiding this comment

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

Reverted the API change for this PR.

@luzhang06
Copy link
Contributor

Hi @baskaryan, can you please help with this PR to move forward? We have addressed your previous comments. Let us know if you have any further comments.

@toothache
Copy link
Contributor

@baskaryan , can you take another look at the change? I have reverted the API change and resolved the conflicts.

Let us know if there's any more concern. Thanks!

@toothache toothache force-pushed the luzhan-docs-update branch from 222fb09 to 8444254 Compare March 20, 2024 17:15
@baskaryan baskaryan enabled auto-merge (squash) March 26, 2024 20:19
@baskaryan
Copy link
Collaborator

some failing tests

auto-merge was automatically disabled March 26, 2024 23:34

Head branch was pushed to by a user without write access

@toothache
Copy link
Contributor

some failing tests

Thanks! The UT has been fixed.

@baskaryan baskaryan enabled auto-merge (squash) March 27, 2024 00:09
@toothache
Copy link
Contributor

Fixing lint.

auto-merge was automatically disabled March 27, 2024 00:18

Head branch was pushed to by a user without write access

@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Mar 27, 2024
@baskaryan baskaryan merged commit f12cb0b into langchain-ai:master Mar 27, 2024
61 checks passed
@toothache
Copy link
Contributor

Thanks so much for the review! @baskaryan @eyurtsev

@eyurtsev
Copy link
Collaborator

Apologies for taking so long!

gkorland pushed a commit to FalkorDB/langchain that referenced this pull request Mar 30, 2024
@ruoccofabrizio @luzhang06
@toothache @teethachee @eyurtsev @baskaryan @gkorland
community[patch]: Microsoft Azure Document Intelligence updates (lang…
868aff7 
…chain-ai#16932)

- **Description:** Update Azure Document Intelligence implementation by
Microsoft team and RAG cookbook with Azure AI Search

---------

Co-authored-by: Lu Zhang (AI) <[email protected]>
Co-authored-by: Yateng Hong <[email protected]>
Co-authored-by: teethache <[email protected]>
Co-authored-by: Lu Zhang <[email protected]>
Co-authored-by: Eugene Yurtsev <[email protected]>
Co-authored-by: Bagatur <[email protected]>
Co-authored-by: Bagatur <[email protected]>
hinthornw pushed a commit that referenced this pull request Apr 26, 2024
@ruoccofabrizio @luzhang06
@toothache @teethachee @eyurtsev @baskaryan @hinthornw
community[patch]: Microsoft Azure Document Intelligence updates (#16932)
320d3c7 
- **Description:** Update Azure Document Intelligence implementation by
Microsoft team and RAG cookbook with Azure AI Search

---------

Co-authored-by: Lu Zhang (AI) <[email protected]>
Co-authored-by: Yateng Hong <[email protected]>
Co-authored-by: teethache <[email protected]>
Co-authored-by: Lu Zhang <[email protected]>
Co-authored-by: Eugene Yurtsev <[email protected]>
Co-authored-by: Bagatur <[email protected]>
Co-authored-by: Bagatur <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@toothache toothache toothache left review comments

@eyurtsev eyurtsev eyurtsev approved these changes

@luzhang06 luzhang06 luzhang06 left review comments

@baskaryan baskaryan baskaryan approved these changes

Assignees

@eyurtsev eyurtsev

Labels
Ɑ: doc loader Related to document loader module (not documentation) 🤖:improvement Medium size change to existing code to handle new use-cases lgtm PR looks good. Use to confirm that a PR is ready for merging. size:XL This PR changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ruoccofabrizio @luzhang06 @toothache @eyurtsev @baskaryan @teethachee