Skip to content

Remove symengine_py feature#42084

Open
orlitzky wants to merge 4 commits into
sagemath:developfrom
orlitzky:remove-symengine-py-feature
Open

Remove symengine_py feature#42084
orlitzky wants to merge 4 commits into
sagemath:developfrom
orlitzky:remove-symengine-py-feature

Conversation

@orlitzky
Copy link
Copy Markdown
Contributor

@orlitzky orlitzky commented Apr 25, 2026

We have a "feature" for the https://github.com/symengine/symengine.py package, used to guard two tests in https://github.com/sagemath/sage/tree/develop/src/sage/symbolic/symengine.py. The functionality that is being tested however lives in the optional package; there is no support for symengine in sagelib. In fact, the same stuff is tested upstream: https://github.com/symengine/symengine.py/blob/master/symengine/tests/test_sage.py

Rather than re-test somebody else's (already tested) code, let's remove these two tests and the feature that guards them.

@orlitzky orlitzky force-pushed the remove-symengine-py-feature branch from bda2ba7 to 7e91afd Compare April 25, 2026 18:08
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 25, 2026

Documentation preview for this PR (built with commit 1ea2e5f; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@cxzhong cxzhong requested a review from tobiasdiez April 26, 2026 11:27
@orlitzky orlitzky force-pushed the remove-symengine-py-feature branch from 7e91afd to 949d450 Compare April 27, 2026 15:21
@orlitzky
Copy link
Copy Markdown
Contributor Author

CC @dimpase just in case, you added this

@dimpase
Copy link
Copy Markdown
Member

dimpase commented Apr 27, 2026

I had a student who wanted to investigate using symengine in Sage, so I added this as a start. Nothing came out of it, though.

Note that symengine and python-symengine are optional backends for sympy, so removing this seems premature

@orlitzky
Copy link
Copy Markdown
Contributor Author

I'm not actually removing anything except two doctests that are duplicated from upstream (and test only upstream code), and then the feature that is not used anywhere else except those two doctests. I'm happy to leave it alone either way though.

Copy link
Copy Markdown
Contributor

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. If there are any concrete plans to use symengine in the near future, then it would make sense to keep the code. Otherwise, it can easily be restored later.

@orlitzky orlitzky force-pushed the remove-symengine-py-feature branch from 949d450 to 0e5fe06 Compare May 5, 2026 16:22
orlitzky added 4 commits May 30, 2026 17:19
This file contains only two doctests, and they test functionality of
the symengine python wrapper:

  https://github.com/symengine/symengine.py

(That is, they do not test anything in Sage itself). There are already
similar tests upstream in the file symengine/tests/test_sage.py,
making these redundant.
The only two tests that required this feature have been removed
because the functionality exists and is tested upstream.
The Sage library does not have any support for this whatsoever, it is
the other way around.
This package has been dropped from pyproject.toml, because it isn't
really a dependency of Sage -- it's the other way around.
@orlitzky orlitzky force-pushed the remove-symengine-py-feature branch from 0e5fe06 to 1ea2e5f Compare May 30, 2026 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants