Skip to content

Remove giac as dependency, and add sagemath_giac as optional dependency #40001

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

Merged
merged 1 commit into from
May 11, 2025

Conversation

tobiasdiez
Copy link
Contributor

Not sure if this change is completely right. It seems to me that there is still a giac expect interface in use, so perhaps giac should now be a runtime dependency instead of build dependency of sagelib? Or is that interface obsolete and should/can be removed?

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • 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

Copy link

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

@user202729
Copy link
Contributor

Or is that interface obsolete and should/can be removed?

Certainly I don't need it, but maybe someone who just-inconveniently-happen to not be able to build sage with giac linked…

Of course that adds to maintenance cost, so I'd be okay with removing it. I think usual procedure requires a one-year deprecation period.

@dimpase
Copy link
Member

dimpase commented Apr 23, 2025

deprecation period only applies to non-bugs. if there is a bug fixed along the way, it can get in right away

@tobiasdiez
Copy link
Contributor Author

@orlitzky what is the status of the giac expect interface? Can this be removed/deprecated?

@orlitzky
Copy link
Contributor

The pexpect interface is used by some other parts of sagelib, but only if you explicitly ask for it. Judging by my own comments, at least the following user-facing bits will still use pexpect rather than the faster library interface:

  • sum(..., algorithm='giac')
  • laplace(..., algorithm='giac')
  • inverse_laplace(..., algorithm='giac')
  • simplify(..., algorithm='giac')

But in all of these cases you have to explicitly ask for giac, and if you don't, giac isn't a dependency.

The answer to the question you asked is, no, it can't be completely dropped yet because those remaining functions need to be ported to sage.libs.giac (i.e. sagemath-giac) first.

But to answer the question you didn't ask, I think you could drop giac as a dependency of sagelib regardless. If someone wants to use algorithm='giac' with any of those functions they can just install giac and it will start working immediately. You'll get an error that suggests this:

TypeError: unable to start giac because the command 'giac --sage' failed: The command was not found or was not executable: giac.


In order to use the Giac interface you need to have Giac installed
and have a program called "giac" in your PATH. You need a giac version
supporting "giac --sage" ( roughly after 0.9.1 of march 2011). Some giac
instructions  and the help's langage depend of you LANG variable. To obtain
inline help for  giac commands, you also need to have the program "cas_help"
in your PATH.

@dimpase
Copy link
Member

dimpase commented Apr 29, 2025

on the sage-distro level, should we relegate giac to the role of a dummy package, as we did for R a while ago? Anyhow, this looks good, and the "required" test-long fails due to some "no space left on device" error, not related to the changes here.

@user202729
Copy link
Contributor

The error message looks… not too actionable? Doesn't help that the online-generated pages for spkg documentation isn't too descriptive on how to actually install the package either, but this particular one doesn't even have sage -i.

@dimpase
Copy link
Member

dimpase commented Apr 29, 2025

sage -i might not even be available - e.g. with a meson build

@user202729
Copy link
Contributor

sage -i might not even be available - e.g. with a meson build

That's another issue, no?

While I use meson myself, I feel it's better to at least be helpful in one circumstance than to be helpful in no circumstance. To avoid maintenance overhead we may just call the feature.requires() framework, then when the suggestion message sage -i is finally changed it will automatically improve.

vbraun pushed a commit to vbraun/sage that referenced this pull request Apr 30, 2025
sagemathgh-40001: Remove giac as dependency, and add sagemath_giac as optional dependency
    
<!-- ^ 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". -->

Not sure if this change is completely right. It seems to me that there
is still a giac expect interface in use, so perhaps giac should now be a
runtime dependency instead of build dependency of sagelib? Or is that
interface obsolete and should/can be removed?

### 📝 Checklist

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

- [ ] The title is concise and informative.
- [ ] The description explains in detail what this PR is about.
- [ ] 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#40001
Reported by: Tobias Diez
Reviewer(s): Dima Pasechnik
vbraun pushed a commit to vbraun/sage that referenced this pull request May 1, 2025
sagemathgh-40001: Remove giac as dependency, and add sagemath_giac as optional dependency
    
<!-- ^ 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". -->

Not sure if this change is completely right. It seems to me that there
is still a giac expect interface in use, so perhaps giac should now be a
runtime dependency instead of build dependency of sagelib? Or is that
interface obsolete and should/can be removed?

### 📝 Checklist

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

- [ ] The title is concise and informative.
- [ ] The description explains in detail what this PR is about.
- [ ] 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#40001
Reported by: Tobias Diez
Reviewer(s): Dima Pasechnik
vbraun pushed a commit to vbraun/sage that referenced this pull request May 4, 2025
sagemathgh-40001: Remove giac as dependency, and add sagemath_giac as optional dependency
    
<!-- ^ 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". -->

Not sure if this change is completely right. It seems to me that there
is still a giac expect interface in use, so perhaps giac should now be a
runtime dependency instead of build dependency of sagelib? Or is that
interface obsolete and should/can be removed?

### 📝 Checklist

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

- [ ] The title is concise and informative.
- [ ] The description explains in detail what this PR is about.
- [ ] 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#40001
Reported by: Tobias Diez
Reviewer(s): Dima Pasechnik
vbraun pushed a commit to vbraun/sage that referenced this pull request May 5, 2025
sagemathgh-40001: Remove giac as dependency, and add sagemath_giac as optional dependency
    
<!-- ^ 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". -->

Not sure if this change is completely right. It seems to me that there
is still a giac expect interface in use, so perhaps giac should now be a
runtime dependency instead of build dependency of sagelib? Or is that
interface obsolete and should/can be removed?

### 📝 Checklist

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

- [ ] The title is concise and informative.
- [ ] The description explains in detail what this PR is about.
- [ ] 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#40001
Reported by: Tobias Diez
Reviewer(s): Dima Pasechnik
vbraun pushed a commit to vbraun/sage that referenced this pull request May 6, 2025
sagemathgh-40001: Remove giac as dependency, and add sagemath_giac as optional dependency
    
<!-- ^ 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". -->

Not sure if this change is completely right. It seems to me that there
is still a giac expect interface in use, so perhaps giac should now be a
runtime dependency instead of build dependency of sagelib? Or is that
interface obsolete and should/can be removed?

### 📝 Checklist

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

- [ ] The title is concise and informative.
- [ ] The description explains in detail what this PR is about.
- [ ] 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#40001
Reported by: Tobias Diez
Reviewer(s): Dima Pasechnik
@vbraun vbraun merged commit 31fd52a into sagemath:develop May 11, 2025
18 of 22 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.

5 participants