Skip to content

[formrecognizer] Fix assertion for signature type fields #21428

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

Merged
catalinaperalta merged 1 commit into from
Oct 27, 2021
Merged

[formrecognizer] Fix assertion for signature type fields #21428

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion sdk/formrecognizer/azure-ai-formrecognizer/tests/testcase.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -773,7 +773,9 @@ def assertDocumentFieldsTransformCorrect(self, document_fields, generated_fields
field_type = expected.type
assert adjust_value_type(field_type) == document_fields[label].value_type
assert expected.confidence == document_fields[label].confidence
assert expected.content == document_fields[label].content
# In the case of content for a signature type field we get '' in expected.content
# vs. None for document_fields[label].content
assert (expected.content == document_fields[label].content) or (expected.content == '' and not document_fields[label].content)
Copy link
Contributor

@kristapratico kristapratico Oct 27, 2021

Choose a reason for hiding this comment

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

what is the testcase that repros this? I'm thinking we should return what the service returns here, but don't quite understand why the content would be "" if there was a value detected.

Copy link
Member Author

@catalinaperalta catalinaperalta Oct 27, 2021

Choose a reason for hiding this comment

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

Value is signed for the custom forms that have a signature. Content is empty...we could do some magic and assign signed to the value...but I have to go look at the code to remember if we do this for any other field that has an empty content.

Copy link
Member Author

@catalinaperalta catalinaperalta Oct 27, 2021

Choose a reason for hiding this comment

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

Or maybe it's a service bug?

Copy link
Contributor

@kristapratico kristapratico Oct 27, 2021

Choose a reason for hiding this comment

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

for selection marks, the service returns "selected" for both value/content, for example. I guess I thought that content would include the signed text or maybe at least "signed".

I suppose it could be hard to translate handwritten/signature text accurately so maybe expecting the text is wrong. But then still wondering why this doesn't follow the pattern that selection marks has.

Copy link
Member Author

@catalinaperalta catalinaperalta Oct 27, 2021

Choose a reason for hiding this comment

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

You bring up a good point about it following the selection mark pattern, I'll bring this up with the service team and see what they think. Should we move forward with this for now? And add an issue to track this meanwhile they answer?

Copy link
Member Author

@catalinaperalta catalinaperalta Oct 27, 2021

Choose a reason for hiding this comment

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

Issue tracking this in either case: #21433

Copy link
Contributor

@kristapratico kristapratico Oct 27, 2021

Choose a reason for hiding this comment

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

sure that works

self.assertDocumentFieldValueTransformCorrect(document_fields[label], expected)

for span, expected_span in zip(document_fields[label].spans or [], expected.spans or []):
Expand Down