Opened 21 months ago

Closed 17 months ago

Last modified 7 weeks ago

#31846 closed enhancement (fixed)

Improve pexpect interface to Singular

Reported by: slelievre Owned by:
Priority: major Milestone: sage-9.5
Component: interfaces Keywords:
Cc: chapoton, dimpase, egourgoulhon, embray, fbissey, mkoeppe, slelievre, tmonteil, tscrim Merged in:
Authors: Frédéric Chapoton Reviewers: Travis Scrimshaw, Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: a10d19d (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description

From #30945.

Change History (9)

comment:1 Changed 21 months ago by tscrim

Cc: tscrim added

comment:2 Changed 19 months ago by chapoton

Status: new → needs_review

comment:3 Changed 19 months ago by tscrim

Reviewers: → Travis Scrimshaw
Status: needs_review → needs_work

Doctest failure also reported by the patchbot: 

sage -t --random-seed=0 src/sage/interfaces/expect.py
**********************************************************************
File "src/sage/interfaces/expect.py", line 921, in sage.interfaces.expect.Expect._eval_line
Failed example:
    singular('2+3')
Expected:
    5
Got:
    Singular crashed -- automatically restarting.
    5
**********************************************************************
1 item had failures:
   1 of  16 in sage.interfaces.expect.Expect._eval_line
    [101 tests, 1 failure, 2.92 s]
----------------------------------------------------------------------
sage -t --random-seed=0 src/sage/interfaces/expect.py  # 1 doctest failed
----------------------------------------------------------------------

comment:4 Changed 18 months ago by git

Commit: 85f65bf368331688ab3b8fe319e318ed046eecdc → a10d19d2cc39f01296d175146d30a1abbe63ab83

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

​7e95128Merge branch 'public/singular_pexpect' in 9.4.rc0
​a10d19dtrac 31846 fix doctest

comment:5 Changed 18 months ago by chapoton

Status: needs_work → needs_review

I have fixed the doctest.

comment:6 Changed 18 months ago by dimpase

Reviewers: Travis Scrimshaw → Travis Scrimshaw, Dima Pasechnik
Status: needs_review → positive_review

lgtm

comment:7 Changed 18 months ago by mkoeppe

Milestone: sage-9.4 → sage-9.5

comment:8 Changed 17 months ago by vbraun

Branch: public/singular_pexpect → a10d19d2cc39f01296d175146d30a1abbe63ab83
Resolution: → fixed
Status: positive_review → closed

comment:9 Changed 7 weeks ago by tornaria

Commit: a10d19d2cc39f01296d175146d30a1abbe63ab83

What does this improve? Will something break if I revert it?

I ask because:

  • I think this change might be the cause of #33907
  • Also, this change makes singular restart every time singular.interrupt() is called. I think that is what the doctest failure in comment:3 above is telling us. I think other interfaces do not restart after interrupt().
  • comments in #30945 seem to give the impression that it was already fixed by the upgrade of cysignals; was this change really needed to fix or improve something?
Note: See TracTickets for help on using tickets.