You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Currently there is some duplication around partial types:
Logic in try_infer_partial_generic_type_from_assignment() essentially duplicates the logic inlined in check_assignment() for None partial types. These two probably can be refactored to the same method if we also update handle_partial_var_type() to not special-case partial None types, and instead consistently return a partial type in lvalue context.
Logic in try_infer_partial_type_from_indexed_assignment() duplicates that in try_infer_partial_type() (there is already a TODO item). This one can be refactored by either pushing the latter a bit down the call stack (closer to check_call()), or by generating a synthetic CallExpr with __setitem__ and passing it to try_infer_partial_type().
The second item may be something to watch out when implementing support for these:
These refactorings sounds like good ideas. I'd prefer not creating temporary node objects if possible, as they tend to increase complexity and sometimes cause trouble with error location reporting.
Currently there is some duplication around partial types:
try_infer_partial_generic_type_from_assignment()
essentially duplicates the logic inlined incheck_assignment()
forNone
partial types. These two probably can be refactored to the same method if we also updatehandle_partial_var_type()
to not special-case partialNone
types, and instead consistently return a partial type in lvalue context.try_infer_partial_type_from_indexed_assignment()
duplicates that intry_infer_partial_type()
(there is already a TODO item). This one can be refactored by either pushing the latter a bit down the call stack (closer tocheck_call()
), or by generating a syntheticCallExpr
with__setitem__
and passing it totry_infer_partial_type()
.The second item may be something to watch out when implementing support for these:
@JukkaL this is probably not something important, but maybe it makes sense to fix this while we are at it?
The text was updated successfully, but these errors were encountered: