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 Azure AI Document Intelligence loader #52

Merged
merged 4 commits into from
Apr 29, 2024
Merged

Add Azure AI Document Intelligence loader #52

merged 4 commits into from
Apr 29, 2024

Conversation

trducng
Copy link
Contributor

@trducng trducng commented Apr 25, 2024

No description provided.

@trducng trducng requested review from taprosoft and lone17 April 25, 2024 14:43
@lone17
Copy link
Contributor

lone17 commented Apr 26, 2024

Looking at this, I think we need to make loaders configurable in flowsettings.py, what do you think ?

@trducng
Copy link
Contributor Author

trducng commented Apr 27, 2024

Looking at this, I think we need to make loaders configurable in flowsettings.py, what do you think ?

@lone17 I did think about this. However, it would means that kotaemon-related component will depend on flowsettings.py, which is meant for ktem. So I think it would be more reasonable to not assume flowsettings.py in kotaemon. And let the pipeline creator for ktem to decide the loader configurations for their pipeline in flowsettings.py

@lone17
Copy link
Contributor

lone17 commented Apr 27, 2024

Looking at this, I think we need to make loaders configurable in flowsettings.py, what do you think ?

@lone17 I did think about this. However, it would means that kotaemon-related component will depend on flowsettings.py, which is meant for ktem. So I think it would be more reasonable to not assume flowsettings.py in kotaemon. And let the pipeline creator for ktem to decide the loader configurations for their pipeline in flowsettings.py

@trducng why would kotaemon's components depend on flowsettings.py ? kotaemon defines the reader class and logic, then flowsettings.py will be used to configure which readers are available just like LLMs, Embedding models, etc.

@trducng
Copy link
Contributor Author

trducng commented Apr 28, 2024

@lone17 Then what you mention is the same as this one.

So I think it would be more reasonable to not assume flowsettings.py in kotaemon. And let the pipeline creator for ktem to decide the loader configurations for their pipeline in flowsettings.py

And that will depend on the settings of ktem and pipeline creator, which doesn't really relate to this PR

@trducng trducng requested a review from taprosoft April 28, 2024 03:15
taprosoft
taprosoft approved these changes Apr 28, 2024
View reviewed changes
Copy link
Collaborator

@taprosoft taprosoft left a comment

Choose a reason for hiding this comment

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

LGTM

@lone17
Copy link
Contributor

lone17 commented Apr 29, 2024

@lone17 Then what you mention is the same as this one.

So I think it would be more reasonable to not assume flowsettings.py in kotaemon. And let the pipeline creator for ktem to decide the loader configurations for their pipeline in flowsettings.py

And that will depend on the settings of ktem and pipeline creator, which doesn't really relate to this PR

yes not related to this one, just a comment, hence I didn't make a review.

@lone17 lone17 merged commit ec11b54 into main Apr 29, 2024
6 checks passed
@lone17 lone17 deleted the feat/azure-di branch April 29, 2024 07:49
phv2312 pushed a commit to phv2312/kotaemon that referenced this pull request Sep 11, 2024
@trducng @phv2312
Add Azure AI Document Intelligence loader (Cinnamon#52)
66133f9 
* Add azureai document intelligence loader

* Add load_data interface to Azure DI

* Bump version

* Access azure credentials from environment variables
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@taprosoft taprosoft taprosoft approved these changes

@lone17 lone17 Awaiting requested review from lone17

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
@trducng @lone17 @taprosoft