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

build: Move Azure's Form Recognizer dependency to extras #5096

Merged
merged 7 commits into from
Jun 12, 2023

Conversation

julian-risch
Copy link
Member

Related Issues

Proposed Changes:

  • Move Azure Form Recognizer dependency from core dependencies to existing file conversion extra because it's only used in one FileConverter called AzureConverter.

How did you test it?

Notes for the reviewer

Checklist

@julian-risch julian-risch requested a review from a team as a code owner June 7, 2023 12:26
@julian-risch julian-risch requested review from silvanocerza and removed request for a team June 7, 2023 12:26
@ZanSara ZanSara requested review from ZanSara and removed request for silvanocerza June 7, 2023 12:54
@ZanSara
Copy link
Contributor

ZanSara commented Jun 7, 2023

Hey @julian-risch! There are no conditional/lazy imports to make after this dependency has moved into the extras? I think right now it doesn't seem like, but only because generalimport is hiding the failure. You may want to wait for #5084 so we're sure it's all working fine.

@coveralls
Copy link
Collaborator

coveralls commented Jun 7, 2023

Pull Request Test Coverage Report for Build 5242021282

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 73 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.1%) to 40.909%

Files with Coverage Reduction New Missed Lines %
agents/base.py 5 91.25%
nodes/search_engine/web.py 9 57.58%
nodes/file_converter/azure.py 59 15.17%
Totals Coverage Status
Change from base Build 5210017042: 0.1%
Covered Lines: 9176
Relevant Lines: 22430

💛 - Coveralls

@julian-risch
Copy link
Member Author

@ZanSara You mean adding sth like what we have for other imports relying on extras?

try:
    from azure.ai.formrecognizer import DocumentAnalysisClient, AnalyzeResult
    from azure.core.credentials import AzureKeyCredential
except ImportError as exc:
    logger.debug(
        "azure.ai.formrecognizer or azure.core.credentials could not be imported. "
        "Run 'pip install farm-haystack[file-conversion]' or 'pip install azure-ai-formrecognizer>=3.2.0b2' to fix this issue."
    )

@ZanSara
Copy link
Contributor

ZanSara commented Jun 8, 2023

Yes! I didn't check right now, but if they're already try-catched like in the snippet you shared, then it's alright

@github-actions github-actions bot added the type:documentation Improvements on the docs label Jun 12, 2023
Copy link
Contributor

@ZanSara ZanSara left a comment

Choose a reason for hiding this comment

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

Great! 🚢

@julian-risch julian-risch merged commit 72fe43a into main Jun 12, 2023
@julian-risch julian-risch deleted the azure-form-recognizer-extra branch June 12, 2023 10:23
@ZanSara ZanSara mentioned this pull request Jun 13, 2023
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants