Upgrading mypy #4062
Upgrading mypy #4062
Conversation
github-actions
bot
added
internal
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
| annotations = job.metadata.annotations or {} | ||
| if step_name := annotations.get(STEP_NAME_ANNOTATION_KEY, None): | ||
| node = nodes[step_name] | ||
| if step_name_annotation_key := annotations.get( |
There was a problem hiding this comment.
The result of this is not a key, but a value. It's the value step_name, which is what the variable was (and should be) IMO
There was a problem hiding this comment.
The variable step_name is already used at this point, which leads to:
src/zenml/integrations/kubernetes/orchestrators/kubernetes_orchestrator_entrypoint.py:161: error: Incompatible types in assignment (expression has type "Any | None", variable has type "str")
You are right though, it is not a key. I have changed it now to step_name_annotation, would it be ok for you?
| if isinstance(obj, BaseModel): | ||
| return obj.model_dump(mode="json") | ||
| elif is_dataclass(obj): | ||
| elif is_dataclass(obj) and not isinstance(obj, type): |
There was a problem hiding this comment.
Why was this necessary? This seems like not just a linting adjustment
There was a problem hiding this comment.
is_dataclass returns True if obj is a dataclass itself or if it's an instance of a dataclass. However, the asdict in the next line only accepts instances and mypy raises:
src/zenml/utils/json_utils.py:118: error: Argument 1 to "asdict" has incompatible type "DataclassInstance | type[DataclassInstance]"; expected "DataclassInstance" [arg-type]
Even in python 3.11, providing a type would lead to an error, so I doubt this changes the functionality. WDYT?
| model_values[k] = new_value | ||
|
|
||
| return cast(V, type(value).model_validate(model_values)) | ||
| return cast(V, type(value).model_validate(model_values)) # type: ignore[redundant-cast] |
There was a problem hiding this comment.
I'm assuming we can't get rid of the cast entirely because this doesn't work on all python versions?
There was a problem hiding this comment.
Not really. If that would be the case, it would lead to an unused-ignore error on the other versions.
As for your question, I was a bit hesitant to remove the redundant casts. We used to do a few tricks with the way we handled pydantic objects and I thought removing the cast might mess up
Documentation Link Check Results❌ Absolute links check failed |
2 participants
Describe changes
I implemented/fixed _ to achieve _.
Pre-requisites
Please ensure you have done the following:
developand the open PR is targetingdevelop. If your branch wasn't based on develop read Contribution guide on rebasing branch to develop.Types of changes