sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.45k stars 482 forks source link

Provide `install_hints` for FriCAS #27323

Closed slel closed 5 years ago

slel commented 5 years ago

Attempting to use an optional package with pexpect interface which is not installed displays whatever the method install_hints provides. This method was not provided by the FriCAS interface, resulting in a unnecessarily difficult to understand error message.

The particular error using limit with algorithm="fricas" in the original report has a different reason, which was fixed in #26068.

Old description:

When FriCAS is not present, we get the following error when trying to compute a limit using FriCAS:

sage: limit(cos(x), x=0, algorithm='fricas')
Traceback (most recent call last)
...
UnboundLocalError: local variable 'l' referenced before assignment

The error message should instead say that FriCAS was not found, and ideally hint at running sage -i fricas in the terminal.

CC: @mantepse @slel

Component: packages: optional

Keywords: FriCAS

Author: Martin Rubey

Branch/Commit: 4fa2a98

Reviewer: Frédéric Chapoton

Issue created by migration from https://trac.sagemath.org/ticket/27323

mantepse commented 5 years ago
comment:1

Great idea! Did you test with or without #26068?

mantepse commented 5 years ago
comment:2

Actually, it would make much more sense to have this message on (failed) startup of FriCAS, that is in interfaces.fricas.FriCAS.__init__ or interfaces.fricas.FriCAS._start.

Unfortunately, it is hard to uninstall fricas.

mantepse commented 5 years ago
comment:3

Wouldn't it be even better to do this for all Expect interfaces?

Oh, this actually exists and is called install_hints. I will provide a fix shortly.

mantepse commented 5 years ago

Branch: u/mantepse/check_for_fricas_when_algorithm_fricas

mantepse commented 5 years ago

New commits:

4828d6cadd install_hints for fricas
mantepse commented 5 years ago

Commit: 4828d6c

mantepse commented 5 years ago

Author: Martin Rubey

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

46a3fcafix typos
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Changed commit from 4828d6c to 46a3fca

mantepse commented 5 years ago

Description changed:

--- 
+++ 
@@ -1,3 +1,9 @@
+Attempting to use an optional package with pexpect interface which is not installed displays whatever the method `install_hints` provides.  This method was not provided by the FriCAS interface, resulting in a unnecessarily difficult to understand error message.
+
+The particular error using `limit` with `algorithm="fricas"` in the original report has a different reason, which was fixed in #26068.
+
+Old description:
+
 When FriCAS is not present, we get the following error when trying
 to compute a limit using FriCAS:
embray commented 5 years ago
comment:8

Ticket retargeted after milestone closed (if you don't believe this ticket is appropriate for the Sage 8.8 release please retarget manually)

fchapoton commented 5 years ago
comment:9

There remains a "Giac" in your "Fricas" hints..

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

4fa2a98Giac -> FriCAS
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Changed commit from 46a3fca to 4fa2a98

mantepse commented 5 years ago
comment:11

Thank you!

fchapoton commented 5 years ago
comment:12

ok, let it be

fchapoton commented 5 years ago

Reviewer: Frédéric Chapoton

vbraun commented 5 years ago

Changed branch from u/mantepse/check_for_fricas_when_algorithm_fricas to 4fa2a98