Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tweak a few tests to pass when giac is not installed #38690

Merged
merged 3 commits into from
Sep 29, 2024

Conversation

orlitzky
Copy link
Contributor

This is one of the prerequisites to make it possible to install sage without giac (#38668). These few tests can be made to work easily without a # needs giac, so let's do that.

Various implementations get it a little (1e-10) different.
…wers

We have an integration test in this file that is looking for the giac
result, but sympy gives an equivalent one. We should accept that, too.
There's one piecewise test that uses algorithm='giac', but it will
soon be possible to install sage without giac. We could make the test
conditional on giac's presence, but the same thing works with sympy,
so let's just use that.
Copy link

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

@orlitzky
Copy link
Contributor Author

As always, the CI fails are bogus/unrelated.

Copy link
Contributor

@dcoudert dcoudert left a comment

Choose a reason for hiding this comment

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

shouldn't you use annotations like # needs sage.libs.giac and/or # needs sympy ?

@orlitzky
Copy link
Contributor Author

There will be a PR where I add a bunch of "needs" tags, but in these few cases I was able to update the expected output to work both with/without giac, so a new "needs" tag isn't necessary. Sympy is a standard package and our "integrate" commands already use it unconditionally; so long as nobody adds a --disable-sympy flag, these tests should pass unconditionally because at least one of the two packages will be installed.

It's unrelated to this issue, but integrating with algorithm='giac' currently uses the slow giac pexpect interface and not the library interface in sage.libs.giac. I've changed this in #38686 to use the better interface, and that PR does add some # needs sage.libs.giac tags (because after the PR, it's using the library interface).

We're also missing a way to detect the giac pexpect interface, but I've added that in #38672. In theory you could have the executable installed but not the library... some binary distros have been known to split packages up like this.

Copy link
Contributor

@dcoudert dcoudert left a comment

Choose a reason for hiding this comment

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

thank you for the explanaition. LGTM.

@orlitzky
Copy link
Contributor Author

Sure, thanks for the review!

@dimpase dimpase mentioned this pull request Sep 25, 2024
2 tasks
vbraun pushed a commit to vbraun/sage that referenced this pull request Sep 27, 2024
sagemathgh-38690: Tweak a few tests to pass when giac is not installed
    
This is one of the prerequisites to make it possible to install sage
without giac (sagemath#38668). These few
tests can be made to work easily without a `# needs giac`, so let's do
that.
    
URL: sagemath#38690
Reported by: Michael Orlitzky
Reviewer(s): David Coudert
@vbraun vbraun merged commit 7b00d7b into sagemath:develop Sep 29, 2024
17 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants