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

src/sage/features/giac.py: add new feature for the giac program #38672

Merged
merged 1 commit into from
Sep 29, 2024

Conversation

orlitzky
Copy link
Contributor

In preparation for adding a --disable-giac option, we add a new feature that detects the presence of the "giac" executable. We already have a feature for sage.libs.giac, but that only guards the libgiac interface; we still have code that runs "giac" behind pexpect. This will allow us to skip those tests when giac is not installed.

In preparation for adding a --disable-giac option, we add a new
feature that detects the presence of the "giac" executable. We already
have a feature for sage.libs.giac, but that only guards the libgiac
interface; we still have code that runs "giac" behind pexpect. This
will allow us to skip those tests when giac is not installed.
Copy link

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

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

@dimpase
Copy link
Member

dimpase commented Sep 24, 2024

should be rebased, but is good to go (I checked, rebases cleanly)

@dimpase dimpase mentioned this pull request Sep 25, 2024
2 tasks
@mkoeppe
Copy link
Contributor

mkoeppe commented Sep 26, 2024

We already have a feature for sage.libs.giac, but that only guards the libgiac interface

@orlitzky Quick correction - this doctest tag has already appeared but the feature definition was missing. #38715 fixes this oversight, please review.

vbraun pushed a commit to vbraun/sage that referenced this pull request Sep 27, 2024
…giac program

    
In preparation for adding a `--disable-giac` option, we add a new
feature that detects the presence of the "giac" executable. We already
have a feature for `sage.libs.giac`, but that only guards the libgiac
interface; we still have code that runs "giac" behind pexpect. This will
allow us to skip those tests when giac is not installed.
    
URL: sagemath#38672
Reported by: Michael Orlitzky
Reviewer(s): Dima Pasechnik
vbraun pushed a commit to vbraun/sage that referenced this pull request Sep 27, 2024
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

The tag `# needs sage.libs.giac` has already appeared in a few places,
but the feature was not defined.

- This complements sagemath#38672 @orlitzky
- Cherry-picked from from sagemath#35095

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->

URL: sagemath#38715
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee
vbraun pushed a commit to vbraun/sage that referenced this pull request Sep 28, 2024
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

The tag `# needs sage.libs.giac` has already appeared in a few places,
but the feature was not defined.

- This complements sagemath#38672 @orlitzky
- Cherry-picked from from sagemath#35095


### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38715
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee
@vbraun vbraun merged commit 1543b87 into sagemath:develop Sep 29, 2024
21 of 22 checks passed
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.

4 participants