Skip to content

Conversation

orlitzky
Copy link
Contributor

@orlitzky orlitzky commented Oct 2, 2024

We turn the libgiac integrator into a no-op if libgiac isn't available. That way, in the default integration routines, it is essentially skipped. If it were tried last, we could literally skip it; but currently it lives between maxima and sympy and cannot be rearanged without messing up some doctests.

We turn the libgiac integrator into a no-op if libgiac isn't
available. That way, in the default integration routines, it is
essentially skipped. If it were tried last, we could literally skip
it; but currently it lives between maxima and sympy and cannot be
rearanged without messing up some doctests.
Copy link

github-actions bot commented Oct 2, 2024

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

@dimpase
Copy link
Member

dimpase commented Oct 3, 2024

Shouldn't the test for giac presence go via src/sage/features/giac ?

@orlitzky
Copy link
Contributor Author

orlitzky commented Oct 3, 2024

It's not using the "giac" executable, only the libgiac interface. I guess we could use the sage.libs.giac feature, but since this code is already going to import sage.libs.giac, it was simpler to catch the ImportError.

Copy link
Member

@dimpase dimpase left a comment

Choose a reason for hiding this comment

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

LGTM

vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 5, 2024
sagemathgh-38756: src/sage/symbolic/integration: make libgiac integration optional
    
We turn the libgiac integrator into a no-op if libgiac isn't available.
That way, in the default integration routines, it is essentially
skipped. If it were tried last, we could literally skip it; but
currently it lives between maxima and sympy and cannot be rearanged
without messing up some doctests.
    
URL: sagemath#38756
Reported by: Michael Orlitzky
Reviewer(s): Dima Pasechnik
vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 9, 2024
sagemathgh-38756: src/sage/symbolic/integration: make libgiac integration optional
    
We turn the libgiac integrator into a no-op if libgiac isn't available.
That way, in the default integration routines, it is essentially
skipped. If it were tried last, we could literally skip it; but
currently it lives between maxima and sympy and cannot be rearanged
without messing up some doctests.
    
URL: sagemath#38756
Reported by: Michael Orlitzky
Reviewer(s): Dima Pasechnik
@vbraun vbraun merged commit 3d5343b into sagemath:develop Oct 12, 2024
20 of 21 checks passed
vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 23, 2024
sagemathgh-38770: Add "needs" tags for giac and libgiac
    
Part of sagemath#38668. If it's going to
be possible to disable giac, we need to guard all of the tests that use
it with either `# needs giac` or `# needs sage.libs.giac`.

I think I've gotten them all. A crude way to test:

1. `git rm -r src/sage/libs/giac` and rebuild to disable sage.libs.giac
2. build sage, and then delete the giac executable to disable the
pexpect interface

If you do these one at a time, it should ensure that the correct tags
are used. (Typically, if giac is missing, neither sage.libs.giac nor the
giac executable will be present, making it very easy to mix up the
tags.)

For bonus points you can undelete `src/sage/libs/giac` after building
but before testing to make sure the "needs" tags in those files are
accurate.

### Dependencies:

* sagemath#38756
* sagemath#38686 (not strictly required,
but it adds a few "needs sage.libs.giac" tags of its own)
    
URL: sagemath#38770
Reported by: Michael Orlitzky
Reviewer(s): Tobias Diez
vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 26, 2024
sagemathgh-38770: Add "needs" tags for giac and libgiac
    
Part of sagemath#38668. If it's going to
be possible to disable giac, we need to guard all of the tests that use
it with either `# needs giac` or `# needs sage.libs.giac`.

I think I've gotten them all. A crude way to test:

1. `git rm -r src/sage/libs/giac` and rebuild to disable sage.libs.giac
2. build sage, and then delete the giac executable to disable the
pexpect interface

If you do these one at a time, it should ensure that the correct tags
are used. (Typically, if giac is missing, neither sage.libs.giac nor the
giac executable will be present, making it very easy to mix up the
tags.)

For bonus points you can undelete `src/sage/libs/giac` after building
but before testing to make sure the "needs" tags in those files are
accurate.

### Dependencies:

* sagemath#38756
* sagemath#38686 (not strictly required,
but it adds a few "needs sage.libs.giac" tags of its own)
    
URL: sagemath#38770
Reported by: Michael Orlitzky
Reviewer(s): Tobias Diez
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