Rework sympy to pymbolic mappers

[alexfikl]

The mappers previously inherited from either the Sympy or SymEngine variants based on USE_SYMENGINE. This meant that you couldn't use them at all for the other one, e.g. if USE_SYMENGINE=True there was no way to convert sympy expressions.

This implements both variants (with some code duplication) and adds to_pymbolic and to_symbolic helper functions that do the actual dispatch based on USE_SYMENGINE.

[alexfikl]

Hmm.. this needs a bit more work. It needs to make a distinction between USE_SYMENGINE and HAS_SYMENGINE..

[alexfikl]

Worth noting: this is pretty breaking because PymbolicToSympy just does sympy now. From what I can tell, these are undocumented classes that are not used by pytential or anyone.. so not a big deal? 😁

[Copilot]

🟡 Changes recommended

The SymEngine symbols=True conversion path does not currently map the imaginary unit I as expected, which can break the new roundtrip behavior covered by tests.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR decouples symbolic↔pymbolic conversion from the active backend (USE_SYMENGINE) by implementing explicit sympy and SymEngine mapper variants and introducing to_pymbolic / to_symbolic (and to_symbolic_expr) helpers that dispatch appropriately. This allows converting expressions for either backend even when the other backend is selected at runtime.

Changes:

• Add to_pymbolic, to_symbolic, and to_symbolic_expr helpers and refactor mapper implementations in sumpy/symbolic.py.
• Refactor call sites across the codebase to use the new helper functions instead of directly instantiating backend-specific mappers.
• Extend roundtrip tests to cover additional known symbols (notably the imaginary unit I).

File summaries

File	Description
sumpy/tools.py	Switch kernel scaling constant conversion to sym.to_pymbolic.
sumpy/test/test_misc.py	Update symbolic roundtrip test to use helpers and include I.
sumpy/test/test_codegen.py	Use to_pymbolic helper for sym→pymbolic conversion in codegen test.
sumpy/symbolic.py	Introduce helper dispatch + implement both sympy and SymEngine mapper variants.
sumpy/qbx.py	Use sym.to_symbolic_expr for strengths conversion.
sumpy/kernel.py	Use sym.to_symbolic_expr(..., symbols=True) for expression/scaling conversion.
sumpy/e2p.py	Use to_pymbolic helper for kernel scaling conversion.
sumpy/codegen.py	Remove local sym→pymbolic mapper and rely on sym.to_pymbolic.
.basedpyright/baseline.json	Update pyright baseline to reflect new/changed typing diagnostics.

Review details

• Files reviewed: 9/9 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[inducer]

Thanks! A few comments/questions below.

[inducer]

Shouldn't it be declared in the mixin instead?

[alexfikl]

Sure. I didn't put it in there because in my head mixins don't carry state, but I guess it doesn't matter that much. pyright is also very upset by the mixin 😢

[inducer]

Well, if methods in the mixin rely on a bit of data, the same class should declare what it's supposed to be. (IMO)

[alexfikl]

Moved in https://github.com/inducer/sumpy/compare/f4c6f95906def291811757bc6aa8e54fb6d09fff..eee22cd78a2bec98513172b412185a449811bd97.

[inducer]

Thx!