-
Notifications
You must be signed in to change notification settings - Fork 3k
Fix for mypy 1.14.1 #41674
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?
Fix for mypy 1.14.1 #41674
Conversation
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.
Pull Request Overview
This PR addresses MyPy 1.14.1 type errors by refining type annotations, unifying import fallbacks, and adding safer attribute access in legacy modules.
- Replaced generic
Sequence
/Mapping
aliases with concreteList
imports where mutation is expected. - Removed intermediate
TypeAlias
assignments and exposed fallback classes/functions directly in legacy adapters. - Wrapped model imports in
TYPE_CHECKING
guards and addedgetattr
/hasattr
checks to avoid runtimeKeyError
s.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
_batch_engine/_run_submitter.py |
Switched Sequence[str] to List[str] and tightened previous type. |
_batch_engine/_engine.py |
Converted pending tasks set to list and updated input type to List . |
_adapters/utils.py , client.py , _flows.py , _errors.py |
Removed TypeAlias exports and streamlined fallback definitions. |
_converters/_models.py |
Added TYPE_CHECKING imports, used hasattr/getattr guards, and adjusted error handling. |
_common/onedp/_types.py |
Introduced Any fallbacks for missing model references. |
_azure/_envs.py |
Simplified nested .get(...) calls for cloud metadata lookup. |
Comments suppressed due to low confidence (3)
sdk/evaluation/azure-ai-evaluation/azure/ai/evaluation/_converters/_models.py:9
- The fallback assignment for
RunStepFunctionToolCall
is insideif TYPE_CHECKING:
and won’t execute at runtime, causing a NameError. Move or replicate the fallback definition outside the TYPE_CHECKING guard.
if TYPE_CHECKING:
sdk/evaluation/azure-ai-evaluation/azure/ai/evaluation/_converters/_models.py:255
- This
arguments
block has been dedented out of theelif
scope, sooptions
may be undefined or stale. It should be indented under thefile_search
branch.
arguments = {
sdk/evaluation/azure-ai-evaluation/azure/ai/evaluation/_legacy/_batch_engine/_engine.py:273
- [nitpick] Converting a
set
of pending tasks to alist
yields arbitrary ordering, which could lead to non-deterministic task scheduling. If order matters, consider a deterministic structure.
pending = list(pending) # Convert set back to list
output = azure_ai_search.get("output", "") | ||
elif hasattr(tool_call.details, "type") and tool_call.details.type == _FABRIC_DATAAGENT: # type: ignore | ||
fabric_dataagent = getattr(tool_call.details, "fabric_dataagent", {}) # type: ignore | ||
output = fabric_dataagent.get("output", "") | ||
except: |
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.
[nitpick] Catching a bare except:
can mask unexpected errors. It's better to catch specific exceptions (e.g., except AttributeError:
or except Exception:
).
except: | |
except (AttributeError, TypeError) as e: | |
# Log the error for debugging purposes | |
print(f"Error occurred while processing tool_call details: {e}") |
Copilot uses AI. Check for mistakes.
AgentsApiResponseFormatOption = Union[ | ||
str, | ||
str, | ||
"_models.AgentsApiResponseFormatMode", | ||
"_models.AgentsApiResponseFormat", | ||
"_models.ResponseFormatJsonSchemaType", | ||
Any, # "_models.AgentsApiResponseFormatMode", | ||
Any, # "_models.AgentsApiResponseFormat", | ||
Any, # "_models.ResponseFormatJsonSchemaType", | ||
] | ||
MessageAttachmentToolDefinition = Union["_models.CodeInterpreterToolDefinition", "_models.FileSearchToolDefinition"] | ||
AgentsApiToolChoiceOption = Union[str, str, "_models.AgentsApiToolChoiceOptionMode", "_models.AgentsNamedToolChoice"] | ||
MessageAttachmentToolDefinition = Union[Any, Any] # "_models.CodeInterpreterToolDefinition", "_models.FileSearchToolDefinition" | ||
AgentsApiToolChoiceOption = Union[str, str, Any, Any] # "_models.AgentsApiToolChoiceOptionMode", "_models.AgentsNamedToolChoice" |
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.
[nitpick] The Union[str, str, ...]
includes str
twice, which is redundant. Removing the duplicate entry will simplify the type definition.
Copilot uses AI. Check for mistakes.
Description
Please add an informative description that covers that changes made by the pull request and link all relevant issues.
If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines