-
Notifications
You must be signed in to change notification settings - Fork 591
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
Msft document intelligence ocr #1184
Msft document intelligence ocr #1184
Conversation
@microsoft-github-policy-service agree company="Microsoft" |
Thanks @gordl! this is a great addition. |
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.
Thanks Gord! This is a great addition. I added some comments mainly on types and docstrings, to make the code easier to extend/customize.
presidio-image-redactor/presidio_image_redactor/document_intelligence_ocr.py
Outdated
Show resolved
Hide resolved
presidio-image-redactor/presidio_image_redactor/document_intelligence_ocr.py
Outdated
Show resolved
Hide resolved
presidio-image-redactor/presidio_image_redactor/document_intelligence_ocr.py
Outdated
Show resolved
Hide resolved
presidio-image-redactor/tests/test_document_intelligence_ocr.py
Outdated
Show resolved
Hide resolved
presidio-image-redactor/tests/integration/test_dicom_image_redactor_engine_integration.py
Show resolved
Hide resolved
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
put LICENSE in the NOTICE file
Optional skip changes based on engine constructor
Fix one missing line
This is ready for a second look. Major changes since the last review including marking as 'skip' any tests that require a key to the DI OCR engine instead of xfail. Providing an incorrect key results in a test failure, all tests pass if you provide a key. Second change I made was to also test the image redactor engine in addition to dicom image redactor, and in order to accommodate that I made some changes to the how tests pass based on image similarity because of different ocr behaviours. |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Looks great, thanks!
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.
Great addition to Presidio! Just a few small changes that should be done prior to merge.
presidio-image-redactor/presidio_image_redactor/document_intelligence_ocr.py
Outdated
Show resolved
Hide resolved
presidio-image-redactor/presidio_image_redactor/document_intelligence_ocr.py
Show resolved
Hide resolved
presidio-image-redactor/presidio_image_redactor/document_intelligence_ocr.py
Outdated
Show resolved
Hide resolved
presidio-image-redactor/tests/integration/test_dicom_image_redactor_engine_integration.py
Outdated
Show resolved
Hide resolved
@niwilso Thank you for your review! I've gone ahead and resolved those comments. |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Change Description
Initial support for document intelligence
Issue reference
This PR fixes issue #1183
Checklist