Repository navigation
Conversation
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
petercorke
left a comment
There was a problem hiding this comment.
Thanks Prajval, this is a well-scoped change. Checking sys.modules in issymbol() is a neat way to handle both import orders, and the isolated-subprocess tests, including the blocked-SymPy one, are exactly the right way to test this.
I merged it with current master locally: 419 passed, 3 skipped, import spatialmath no longer loads SymPy, and the isscalar/isvector timings are unchanged.
One thing to fix before merging:
pi()docstring insymbolic.py. The whole docstring has been indented by an extra 4 spaces, and a strayfrom typing import TYPE_CHECKINGhas landed inside therunblockexample. That ends the block early, so the docs will render incorrectly, and CI doesn't catch it because the sphinx job is skipped. It looks like a find-and-replace that went astray; restoring the original docstring should fix it.
Two smaller things, neither blocking:
- On
master,from spatialmath.base.symbolic import *madesympy,Symbolandsymtypeavailable in the importing namespace (and assmb.sympy,smb.Symbol,smb.symtype) whenever SymPy was installed. With this PR those three names are no longer exported that way, because star-imports don't go through the module-level__getattr__. Explicitfrom spatialmath.base.symbolic import Symbol, symtypestill works. I couldn't find any users in RTB, MVTB, bdsim or Swift, but it's worth a line in the description. - Codecov shows the
ImportErrorfallbacks intr2xytanddetaren't exercised. A test with an object-dtype array under the SymPy blocker would cover both.
Approving, please fix the docstring before merge.
Part of #234, covering the SymPy half only. The SciPy work remains separate in #244.
import spatialmathno longer loads SymPy just to check scalar types.issymbol()checks the already-loaded module, and scalar/vector checks use it dynamically so importing SymPy after SpatialMath works too.The other eager SymPy imports in vector norms, object-matrix determinants and 2D pose extraction are deferred. Explicit symbolic helpers load the optional dependency when called; explicit access to
Symbolandsymtyperemains available.Added nine isolated-process test cases covering numeric imports and operations, both import orders, symbolic arithmetic and transforms, explicit helpers and legacy aliases, and numeric use with SymPy blocked.
Validation: Python 3.10/Linux,
MPLBACKEND=Agg python -m pytest tests/ -p no:xvfb -q: 389 passed, 3 skipped, 18 subtests passed. No timing claim is made here. Other Python versions and platforms were not tested.