Opened 5 years ago

Closed 5 years ago

Last modified 4 years ago

#15631 closed defect (fixed)

Random failures in

Reported by: vbraun Owned by:
Priority: major Milestone: sage-6.3
Component: interfaces Keywords: pexpect echo singular
Cc: Merged in:
Authors: Volker Braun Reviewers: Nils Bruin
Report Upstream: N/A Work issues:
Branch: 2528fb3 (Commits) Commit:
Dependencies: Stopgaps:


As reported on

 File "src/sage/sandpiles/", line 4684, in sage.sandpiles.sandpile.complete_sandpile
Failed example:
      File "multi_polynomial_libsingular.pyx", line 912, in sage.rings.polynomial.multi_polynomial_libsingular.MPolynomialRing_libsingular.__call__ (sage/rings/polynomial/multi_polynomial_libsingular.cpp:7912)
    TypeError: Could not find a mapping of the passed element to this ring.
(same place but different error:)
      File "multi_polynomial_libsingular.pyx", line 807, in sage.rings.polynomial.multi_polynomial_libsingular.MPolynomialRing_libsingular.__call__ (sage/rings/polynomial/multi_polynomial_libsingular.cpp:6207)
      File "/usr/local/sage/sage-dev/local/lib/python2.7/site-packages/sage/interfaces/", line 1245, in __repr__
        elif self.type() == 'matrix':
      File "/usr/local/sage/sage-dev/local/lib/python2.7/site-packages/sage/interfaces/", line 1976, in type
    AttributeError: 'NoneType' object has no attribute 'group'

This is because the singular pexpect interface has subtle bugs. The easy fix is to rewrite it with disabled terminal echo.

Change History (22)

comment:1 Changed 5 years ago by vbraun

  • Branch set to u/vbraun/sandpile

comment:2 Changed 5 years ago by vbraun

  • Commit set to 8d1705962533130ea1b0c1d7a533df3a06a4b06e

Annoyingly, the pexpect stuff in Sage is built around terminal echo: everything up to the first return is presumed echo and ignored. So I had to revert to a hack...

Last 10 new commits:

8d17059Disable terminal echo in the Singular interface
46eae63Updated Sage version to 6.1.beta3
8b85f8dTrac #15444: Two algorithms for k-charge do not give same answer
8df6474Fixed bug in k-charge implementation for J-algorithm
26cd2c9Trac #15596: Sdist fails with capital-P illow
c7c0106sage-sdist: copy upstream tarballs using sage-spkg
321a9adMerge remote-tracking branch 'trac/u/ohanar/pillow' into t/15596/sdist_fails_with_capital_p_illow
7aee9ccTrac #15602: Fix documentation
3d319adFixed docbuild.
3e449b2Trac #15561: freetype 2.3.5 is too old for some fonts of current systems.

comment:3 Changed 5 years ago by vbraun

Its even worse, it seems one can't work around this without overriding _eval_line. Which is a mess...

comment:4 Changed 5 years ago by git

  • Commit changed from 8d1705962533130ea1b0c1d7a533df3a06a4b06e to 773c907730dafd290b626588666a50053ed35777

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

773c907override the _eval_line method as well

comment:5 Changed 5 years ago by vbraun

  • Status changed from new to needs_review

Now all tests pass for me (at least once, please try in loop)

comment:6 Changed 5 years ago by nbruin

Patch looks good and I haven't observed the problem any more. The patchbot doesn't see real problems. You can browse through the various plugin complaints yourself.

Question: as far as pexpect interface goes, singular is pretty vanilla if I'm not mistaken. That makes me think that whatever approach you have taken to make no-echo work with singular might apply more generally. Would it be doable to include the new way of handling "noecho" in our pexpect wrapper and select it with another flag to be set on startup? Then the code could be reused for other interfaces too.

comment:7 Changed 5 years ago by vbraun

  • Commit changed from 773c907730dafd290b626588666a50053ed35777 to f3e08b9ffdbbbe9a7d4fba6c7e6b1a76c6b68b19

I've added a terminal_echo=<bool> optional keyword argument to Expect, this saves some code but makes the _eval_line method even more ugly.... oh well.

New commits:

f3e08b9make terminal echo optional in Expect class

comment:8 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:9 Changed 5 years ago by vbraun

Nils, are you still reviewing this ticket?

comment:10 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:11 Changed 5 years ago by rws

  • Status changed from needs_review to needs_work
  • Work issues set to rebase

comment:12 Changed 5 years ago by rws

This also affects some patchbot doctesting, e.g.

comment:13 Changed 5 years ago by rws

  • Branch changed from u/vbraun/sandpile to u/rws/sandpile

comment:14 Changed 5 years ago by rws

  • Commit changed from f3e08b9ffdbbbe9a7d4fba6c7e6b1a76c6b68b19 to 2528fb3579a0503f765f787384158f2478e00ed2
  • Status changed from needs_work to needs_review

New commits:

2528fb3Merge branch 'develop' into t/15631/sandpile

comment:15 Changed 5 years ago by rws

  • Status changed from needs_review to positive_review

Ticket was already reviewed. Patchbot is happy also with current version.

comment:16 Changed 5 years ago by rws

  • Reviewers set to Nils Bruin
  • Work issues rebase deleted

Hope you don't mind me setting the name.

comment:17 Changed 5 years ago by vbraun

  • Authors set to Volker Braun

comment:18 Changed 5 years ago by vbraun

  • Branch changed from u/rws/sandpile to 2528fb3579a0503f765f787384158f2478e00ed2
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:19 Changed 5 years ago by leif

  • Commit 2528fb3579a0503f765f787384158f2478e00ed2 deleted
  • Keywords pexpect echo singular added

It would have been nice if you mentioned pexpect in the ticket's title...

comment:20 Changed 4 years ago by jdemeyer

What's the precise motivation for

                    if out == '':   # match bug with echo
                        out = line

This is breaking a fix for #10476.

comment:21 Changed 4 years ago by jdemeyer

Follow-up: #17719

comment:22 Changed 4 years ago by vbraun

Don't remember, though clearly must have been because something further down is unhappy with an empty string.

Note: See TracTickets for help on using tickets.