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

[formrecognizer] update samples to train a model where a model_id is needed #18789

Merged
merged 7 commits into from
May 19, 2021
Merged

[formrecognizer] update samples to train a model where a model_id is needed #18789

merged 7 commits into from
May 19, 2021

Conversation

kristapratico
Copy link
Member

Resolves #18465

  • Adds the option to provide a model_id or train a model for any sample that requires a model_id to run.
  • Improves docs to point to the sample training files location.
  • Removes the last of our static environment variables so samples no longer need to rely on an existing FR resource. This means I don't have to maintain said resource and periodically delete models from it. :)

@kristapratico kristapratico self-assigned this May 18, 2021
@kristapratico kristapratico requested a review from benbp as a code owner May 18, 2021 16:50
@kristapratico kristapratico removed the request for review from benbp May 18, 2021 16:50
maririos
maririos reviewed May 18, 2021
View reviewed changes
...es/async_samples/sample_differentiate_output_models_trained_with_and_without_labels_async.py
@@ -13,6 +13,7 @@
This sample demonstrates the differences in output that arise when begin_recognize_custom_forms
is called with custom models trained with labels and without labels. The models used in this
sample can be created in sample_train_model_with_labels_async.py and sample_train_model_without_labels_async.py
using the training files found here: https://aka.ms/azsdk/formrecognizer/sampletrainingfiles
Copy link
Member

Choose a reason for hiding this comment

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

smart!

maririos
maririos reviewed May 18, 2021
View reviewed changes
sdk/formrecognizer/tests.yml
@@ -30,19 +30,3 @@ stages:
TEST_MODE: 'RunLiveNoRecord'
AZURE_SKIP_LIVE_RECORDING: 'True'
AZURE_TEST_RUN_LIVE: 'true'
# EnvVars for samples to run. TODO: These use an existing FR resource. Fix samples so they don't have to
Copy link
Member

Choose a reason for hiding this comment

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

woohooooo

maririos
maririos reviewed May 18, 2021
View reviewed changes
sdk/formrecognizer/azure-ai-formrecognizer/samples/async_samples/sample_copy_model_async.py Outdated
@@ -23,6 +26,9 @@
3) AZURE_FORM_RECOGNIZER_TARGET_ENDPOINT - the endpoint to your target Form Recognizer resource.
4) AZURE_FORM_RECOGNIZER_TARGET_KEY - your target Form Recognizer API key
5) AZURE_SOURCE_MODEL_ID - the model ID from the source resource to be copied over to the target resource.
- OR -
CONTAINER_SAS_URL - The shared access signature (SAS) Url of your Azure Blob Storage container with your forms.
A model will be trained and used to to run the sample.
Copy link
Member

Choose a reason for hiding this comment

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

duplicate to to it is in other places too

Copy link
Member

Choose a reason for hiding this comment

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

And not related, but in my head, when I read it in Spanish it sounded like: https://www.youtube.com/watch?v=5AkDqm-cEgg

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks for catching this! love the song :)

iscai-msft
iscai-msft approved these changes May 18, 2021
View reviewed changes
sdk/formrecognizer/azure-ai-formrecognizer/samples/async_samples/sample_copy_model_async.py Outdated
from azure.core.credentials import AzureKeyCredential
from azure.ai.formrecognizer.aio import FormTrainingClient

source_endpoint = os.environ["AZURE_FORM_RECOGNIZER_ENDPOINT"]
source_key = os.environ["AZURE_FORM_RECOGNIZER_KEY"]
target_endpoint = os.environ["AZURE_FORM_RECOGNIZER_TARGET_ENDPOINT"]
target_key = os.environ["AZURE_FORM_RECOGNIZER_TARGET_KEY"]
source_model_id = os.environ["AZURE_SOURCE_MODEL_ID"]
source_model_id = os.getenv("AZURE_SOURCE_MODEL_ID") or custom_model_id
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: i think you can do os.getenv("AZURE_SOURCE_MODEL_ID", custom_model_id)

Copy link
Member Author

Choose a reason for hiding this comment

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

good catch! I was using os.environ before and then missed this when I switched to getenv :)

catalinaperalta
catalinaperalta approved these changes May 18, 2021
View reviewed changes
Copy link
Member

@catalinaperalta catalinaperalta left a comment

Choose a reason for hiding this comment

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

Agree with the other nits. Aside from that this looks great! I think the samples will be so much easier to use now as well! 🎉

@kristapratico kristapratico merged commit 66dd15e into master May 19, 2021
@kristapratico kristapratico deleted the fr-samples-option branch May 19, 2021 16:20
iscai-msft added a commit to iscai-msft/azure-sdk-for-python that referenced this pull request May 28, 2021
@iscai-msft
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-python …
db30566 
…into protocol_base

* 'master' of https://github.com/Azure/azure-sdk-for-python: (365 commits)
  Agrifood/test infra (Azure#18779)
  [formrecognizer] update samples to train a model where a model_id is needed (Azure#18789)
  replace response urls too (Azure#18758)
  [AutoRelease] t2-resource-2021-05-19-78417 (Azure#18807)
  [pipeline]update autorest version for pipeline (Azure#18782)
  [AutoRelease] t2-digitaltwins-2021-05-18-25594(wave4) (Azure#18786)
  [AutoRelease] t2-hybridkubernetes-2021-05-19-49364(wave4) (Azure#18802)
  Added Async APIs and Samples for attestation SDK (Azure#18735)
  [AVA] Updating links in package ReadMe (Azure#18800)
  Sync eng/common directory with azure-sdk-tools for PR 1585 (Azure#18793)
  Increment version for textanalytics releases (Azure#18797)
  remove PHI with multiple actions due to known service error (Azure#18794)
  [textanalyitcs] switch to westus2 region (Azure#18791)
  Remove implicit dependency on successful completion of verify-links (Azure#18277)
  Add Testing of Conda Artifacts (Azure#18478)
  reset continuation token (Azure#18772)
  pipline prepare (Azure#18755)
  [AutoRelease] t2-machinelearningcompute-2021-05-14-14426(wave4) (Azure#18750)
  [AutoRelease] t2-iothubprovisioningservices-2021-05-14-29542(wave4) (Azure#18748)
  [AutoRelease] t2-agfood-2021-05-17-53436 (Azure#18768)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@catalinaperalta catalinaperalta catalinaperalta approved these changes

@maririos maririos maririos approved these changes

@iscai-msft iscai-msft iscai-msft approved these changes

@mssfang mssfang Awaiting requested review from mssfang

@witemple-msft witemple-msft Awaiting requested review from witemple-msft

Assignees

@kristapratico kristapratico

Labels
Cognitive - Form Recognizer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[formrecognizer] add train a model or use existing model option to samples
4 participants
@kristapratico @catalinaperalta @maririos @iscai-msft