-
Notifications
You must be signed in to change notification settings - Fork 50
Update DI to CU converter for GA #131
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
base: main
Are you sure you want to change the base?
Conversation
| elif value_type == "number": | ||
| try: | ||
| di_label["valueNumber"] = float(value.get("content")) # content can be easily converted to a float | ||
| content_val = value.get("content") |
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.
Hi @aainav269, I encountered some errors when I tried to convert fields labeled by region in DI studio which would not have content. I'm wondering if we encountered this error before and if we are good to set value as None.
| print("Successfully refreshed token") | ||
| return current_token | ||
|
|
||
| def build_analyzer(credential, current_token, host, api_version, subscriptionKey) -> str: |
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.
Hi @aainav269, I'm removing this function because this is not used. Do we intend to use it somewhere?
| analyzer_id = build_analyzer(credential, current_token, host, api_version, subscription_key) | ||
| url = f"{host}/contentunderstanding/analyzers/{analyzer_id}:analyze?api-version={api_version}" | ||
| # Use prebuilt-layout analyzer directly - no need to create a custom analyzer | ||
| url = f"{host.rstrip('/')}/contentunderstanding/analyzers/prebuilt-layout:analyzeBinary?api-version={api_version}" |
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.
In Joe's PR, the prebuilt-read model was used. I will need to confirm which to use, prebuilt-layout or prebuilt-read.
|
|
||
| # imports from same project | ||
| from constants import CU_API_VERSION, MAX_FIELD_LENGTH, VALID_CU_FIELD_TYPES | ||
| from constants import CU_API_VERSION, MAX_FIELD_LENGTH, VALID_CU_FIELD_TYPES, COMPLETION_MODEL, EMBEDDING_MODEL |
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.
Hi @aainav269, I found we only validate the length of field name and do not check/normalize the field name by our current field limitation. It seems like we also don't check/remove the field format. Do you recall the discussion of field name normalization in this tool?
Purpose
Does this introduce a breaking change?
Pull Request Type
What kind of change does this Pull Request introduce?
How to Test
What to Check
Verify that the following are valid
Other Information