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

feat: add Form Recognizer V3.0 #1269

Merged
merged 11 commits into from
Mar 7, 2022

Conversation

serena-ruan
Copy link
Contributor

No description provided.

@serena-ruan
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@codecov-commenter
Copy link

codecov-commenter commented Nov 29, 2021

Codecov Report

Merging #1269 (901e8b2) into master (58dd01f) will decrease coverage by 0.19%.
The diff coverage is 54.76%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1269      +/-   ##
==========================================
- Coverage   84.81%   84.62%   -0.20%     
==========================================
  Files         287      288       +1     
  Lines       14250    14292      +42     
  Branches      719      718       -1     
==========================================
+ Hits        12086    12094       +8     
- Misses       2164     2198      +34     
Impacted Files Coverage Δ
...ft/azure/synapse/ml/cognitive/FormRecognizer.scala 75.92% <0.00%> (-6.08%) ⬇️
.../azure/synapse/ml/cognitive/FormRecognizerV3.scala 67.64% <67.64%> (ø)
...crosoft/azure/synapse/ml/io/http/HTTPClients.scala 58.33% <0.00%> (-18.34%) ⬇️
...ala/org/apache/spark/ml/param/DataFrameParam.scala 70.83% <0.00%> (-16.67%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 58dd01f...901e8b2. Read the comment docs.

@serena-ruan
Copy link
Contributor Author

Pending on form recognizer team fixing custom model of V3.0

@serena-ruan
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@serena-ruan
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@serena-ruan
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@serena-ruan serena-ruan marked this pull request as ready for review March 1, 2022 10:46
Copy link
Collaborator

@mhamilton723 mhamilton723 left a comment

Choose a reason for hiding this comment

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

Wooo woo! Looking awesome! A few nits and a rename request. Also we should remember to put this in website Transformers section, though feel free to do in follow up

getUrlParams.asInstanceOf[Array[ServiceParam[Any]]];
// This semicolon is needed to avoid argument confusion
{ row: Row =>
val base = getUrl + s"${getValue(row, prebuiltModelId)}:analyze?api-version=2021-09-30-preview"
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit, perhaps make api_version a param with this as default


object AnalyzeDocumentV3 extends ComplexParamsReadable[AnalyzeDocumentV3]

class AnalyzeDocumentV3(override val uid: String) extends CognitiveServicesBaseNoHandler(uid)
Copy link
Collaborator

Choose a reason for hiding this comment

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

are we able to call this AnalyzeDocument and rename previous to AnalyzeDocumentV2? Thats what we did for TA update


test("Prebuilt-receipt Basic Usage") {
val result1 = modelsTest(analyzeReceipts, imageDf2, false)
resultAssert(result1, "Contoso\nContoso\n123 Main Street\nRedmond, WA 98052\n123-456-7890\n" +
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: theres a bit of duplication in these tests (each assert happens twice), perhaps there's a nice way to reduce copy-paste here. Dont spend too much time worrying about it though :)

@serena-ruan
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@serena-ruan
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Collaborator

@mhamilton723 mhamilton723 left a comment

Choose a reason for hiding this comment

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

What a hero PR

@serena-ruan serena-ruan merged commit ef27015 into microsoft:master Mar 7, 2022
@serena-ruan serena-ruan deleted the serena/updateFormVersion branch March 7, 2022 03:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants