symforce-org / symforce

Fast symbolic computation, code generation, and nonlinear optimization for robotics
https://symforce.org
Apache License 2.0
1.41k stars 145 forks source link

Assert Matrix shape is correct when copying matrix #290

Closed bradley-solliday-skydio closed 1 year ago

bradley-solliday-skydio commented 1 year ago

Previously it was possible to construct a sf.V2 with sf.V3(sf.V2()). Not only is this confusing, but it can also be used to trick the type checker into thinking a matrix is of a type it is not (for example, mypy assumes sf.V3(sf.V2()) is a sf.V3).

An example where this change is useful is, for a = sf.M23() and b = sf.M34(), sf.M24(a * b). This is because mypy cannot tell that a * b has type sf.M24. With this change, not only does wrapping the expression in sf.M24 communicate this fact to mypy, but it also performs a runtime check that it does in fact have correct shape.

In matrix.py, had to change several methods to use Matrix instead of self.__class__ or cls to construct a new matrix object when the new matrix was not guarenteed to have the same shape.

Topic: check_shape_copy_construct

bradley-solliday-skydio commented 1 year ago

Reviews in this chain: └https://github.com/symforce-org/symforce/pull/290 Assert Matrix shape is correct when copying matrix

bradley-solliday-skydio commented 1 year ago
# head base diff date summary
0 9b0b32b6 474f1bd5 diff Jan 5 15:57 PM 2 files changed, 11 insertions(+), 5 deletions(-)