Fix placeholder deep copy #611
Conversation
Co-authored-by: James Hilton-Balfe <[email protected]>
src/betterproto/__init__.py
Outdated
|
|
||
| # We can't simply use object() here because pydantic automatically performs deep-copy of mutable default values | ||
| # See #606 | ||
| PLACEHOLDER: Placeholder = Placeholder() |
There was a problem hiding this comment.
| PLACEHOLDER: Placeholder = Placeholder() | |
| PLACEHOLDER: Any = Placeholder() |
There was a problem hiding this comment.
I'm not sure to understand, is there any reason for this?
There was a problem hiding this comment.
It's so it can be used anywhere and the type checker won't complain
There was a problem hiding this comment.
I think the type checker complains anyway: with mypy, your changes turns:
src/betterproto/__init__.py:224: error: Argument "default" to "field" has incompatible type "Placeholder | None"; expected "Field[Any]" [arg-type]
into
src/betterproto/__init__.py:224: error: Argument "default" to "field" has incompatible type "Any | None"; expected "Field[Any]" [arg-type]
It is coherent since if x has type T and y has type Any, then x = y should not be a valid operation. Wouldn't it be better to add # type: ignore on the lines where it is needed? I think the line 224 is the only one with this kind of error.
There was a problem hiding this comment.
It should still be an Any, but the type ignore would be appreciated
There was a problem hiding this comment.
I updated the PR
|
Thanks! |
Summary
[EDIT] Fix #606
Checklist