Opened 2 years ago

Closed 2 years ago

Last modified 16 months ago

#28622 closed defect (fixed)

Patchbot and Python 3 doctest failures

Reported by: jhpalmieri Owned by:
Priority: major Milestone: sage-9.0
Component: python3 Keywords: python3
Cc: Merged in:
Authors: John Palmieri Reviewers: Frédéric Chapoton, Eric Gourgoulhon
Report Upstream: N/A Work issues:
Branch: 56e1642 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by egourgoulhon)

With a Python 3 patchbot and a Ubuntu 18.04 computer running make ptestlong, there are doctest failures in three files:

sage -t --long src/sage/repl/attach.py  # 4 doctests failed
sage -t --long src/sage/libs/singular/function.pyx  # 1 doctest failed
sage -t --long src/sage/numerical/backends/generic_backend.pyx  # 2 doctests failed

This can be replicated by hand. cd to SAGE_ROOT for a Sage built with Python 3, run Python, and do

>>> import subprocess, os, sys
>>> tee = subprocess.Popen(["tee", 'LOGFILE.log'], stdin=subprocess.PIPE)
>>> os.dup2(tee.stdin.fileno(), sys.stdout.fileno())
>>> os.dup2(tee.stdin.fileno(), sys.stderr.fileno())
>>> os.system('./sage -tp src/sage/repl/attach.py src/sage/libs/singular/function.pyx src/sage/numerical/backends/generic_backend.pyx')

Note that in a fresh Python session, the following does not lead to errors:

>>> import os
>>> os.system('./sage -tp src/sage/repl/attach.py src/sage/libs/singular/function.pyx src/sage/numerical/backends/generic_backend.pyx')

Change History (29)

comment:1 Changed 2 years ago by jhpalmieri

The failures:

sage -t --long src/sage/repl/attach.py
**********************************************************************
File "src/sage/repl/attach.py", line 36, in sage.repl.attach
Failed example:
    try:
        attach(src)
    except Exception:
        traceback.print_exc()
Expected:
    Traceback (most recent call last):
    ...
        exec(preparse_file(f.read()) + "\n", globals)
      File "<string>", line 3, in <module>
    ValueError: third
Got:
    <BLANKLINE>
**********************************************************************
File "src/sage/repl/attach.py", line 45, in sage.repl.attach
Failed example:
    detach(src)
Expected nothing
Got:
    Traceback (most recent call last):
      File "<doctest sage.repl.attach[10]>", line 2, in <module>
        attach(src)
      File "sage/misc/lazy_import.pyx", line 354, in sage.misc.lazy_import.LazyImport.__call__ (build/cythonized/sage/misc/lazy_import.c:3697)
        return self.get_object()(*args, **kwds)
      File "/Users/jpalmier/Desktop/Sage/sage_builds/TESTING/sage-9.0.beta1/local/lib/python3.7/site-packages/sage/repl/attach.py", line 356, in attach
        load(filename, globals(), attach=True)
      File "/Users/jpalmier/Desktop/Sage/sage_builds/TESTING/sage-9.0.beta1/local/lib/python3.7/site-packages/sage/repl/load.py", line 272, in load
        exec(preparse_file(f.read()) + "\n", globals)
      File "<string>", line 3, in <module>
    ValueError: third
**********************************************************************
File "src/sage/repl/attach.py", line 48, in sage.repl.attach
Failed example:
    try:
        attach(src)
    except Exception:
        traceback.print_exc()
Expected:
    Traceback (most recent call last):
    ...
        exec(code, globals)
      File ".../foobar.sage....py", line ..., in <module>
        raise ValueError("third")   # this should appear in the source snippet
    ValueError: third
Got:
    <BLANKLINE>
**********************************************************************
File "src/sage/repl/attach.py", line 58, in sage.repl.attach
Failed example:
    detach(src)
Expected nothing
Got:
    Traceback (most recent call last):
      File "<doctest sage.repl.attach[13]>", line 2, in <module>
        attach(src)
      File "sage/misc/lazy_import.pyx", line 354, in sage.misc.lazy_import.LazyImport.__call__ (build/cythonized/sage/misc/lazy_import.c:3697)
        return self.get_object()(*args, **kwds)
      File "/Users/jpalmier/Desktop/Sage/sage_builds/TESTING/sage-9.0.beta1/local/lib/python3.7/site-packages/sage/repl/attach.py", line 356, in attach
        load(filename, globals(), attach=True)
      File "/Users/jpalmier/Desktop/Sage/sage_builds/TESTING/sage-9.0.beta1/local/lib/python3.7/site-packages/sage/repl/load.py", line 266, in load
        exec(code, globals)
      File "/Users/jpalmier/.sage/temp/jpalmieri138.local/85343/foobar.sageyh9c7b26.py", line 7, in <module>
        raise ValueError("third")   # this should appear in the source snippet
    ValueError: third
**********************************************************************
1 item had failures:
   4 of  16 in sage.repl.attach
    [129 tests, 4 failures, 2.43 s]

and

sage -t --long src/sage/libs/singular/function.pyx
**********************************************************************
File "src/sage/libs/singular/function.pyx", line 1314, in sage.libs.singular.function.SingularFunction.__call__
Failed example:
    G= Ideal(I.groebner_basis())
Expected nothing
Got:
    RuntimeError: Error raised calling singular function
    Exception ignored in: 'sage.libs.singular.function.LibraryCallHandler.handle_call'
    RuntimeError: Error raised calling singular function
**********************************************************************
1 item had failures:
   1 of  24 in sage.libs.singular.function.SingularFunction.__call__
    [302 tests, 1 failure, 1.63 s]

and

File "src/sage/numerical/backends/generic_backend.pyx", line 181, in sage.numerical.backends.generic_backend.GenericBackend._test_add_variables
Failed example:
    sig_on_count() # check sig_on/off pairings (virtual doctest)
Expected:
    0
Got:
    NotImplementedError
    Exception ignored in: 'sage.numerical.backends.generic_backend.GenericBackend.ncols'
    NotImplementedError: 
    0
**********************************************************************
File "src/sage/numerical/backends/generic_backend.pyx", line 646, in sage.numerical.backends.generic_backend.GenericBackend._test_add_linear_constraints
Failed example:
    sig_on_count() # check sig_on/off pairings (virtual doctest)
Expected:
    0
Got:
    NotImplementedError
    Exception ignored in: 'sage.numerical.backends.generic_backend.GenericBackend.nrows'
    NotImplementedError: 
    0
**********************************************************************
2 items had failures:
   1 of   4 in sage.numerical.backends.generic_backend.GenericBackend._test_add_linear_constraints
   1 of   4 in sage.numerical.backends.generic_backend.GenericBackend._test_add_variables
    [89 tests, 2 failures, 1.04 s]

comment:2 Changed 2 years ago by jhpalmieri

  • Component changed from PLEASE CHANGE to python3
  • Type changed from PLEASE CHANGE to defect

comment:3 Changed 2 years ago by jhpalmieri

A quicker way to get the failures:

>>> from sage_patchbot.patchbot import Tee
>>> with Tee('/path/to/logfile', time=True, timeout=40):
...     os.system('/path/to/SAGE_ROOT/sage -tp 3 --long src/sage/repl/attach.py src/sage/numerical/backends/generic_backend.pyx src/sage/libs/singular/function.pyx')
Last edited 2 years ago by jhpalmieri (previous) (diff)

comment:4 Changed 2 years ago by jhpalmieri

  • Description modified (diff)

comment:5 Changed 2 years ago by jhpalmieri

Could the problem be the inheritability (or not) of the file descriptors in Tee? In any case, in sage_patchbot.patchbot, if I comment out these lines, the doctests don't fail:

  • sage_patchbot/patchbot.py

    diff --git a/sage_patchbot/patchbot.py b/sage_patchbot/patchbot.py
    index b411c7c..2994db1 100755
    a b class Tee(object): 
    142142        self._saved = os.dup(sys.stdout.fileno()), os.dup(sys.stderr.fileno())
    143143        self.tee = subprocess.Popen(["tee", self.filepath],
    144144                                    stdin=subprocess.PIPE)
    145         os.dup2(self.tee.stdin.fileno(), sys.stdout.fileno())
    146         os.dup2(self.tee.stdin.fileno(), sys.stderr.fileno())
     145        # os.dup2(self.tee.stdin.fileno(), sys.stdout.fileno())
     146        # os.dup2(self.tee.stdin.fileno(), sys.stderr.fileno())
    147147        if self.time:
    148148            print(now_str())
    149149            self.start_time = time.time()

Of course also the log file doesn't get written.

comment:6 Changed 2 years ago by jhpalmieri

  • Description modified (diff)

comment:7 Changed 2 years ago by jhpalmieri

  • Description modified (diff)

comment:8 Changed 2 years ago by jhpalmieri

Could there be some connection to the use of Python's traceback module? I'm trying to find common threads among the three files with failures, and I'm not getting very far, but if I make this change, then tests pass in repl/attach.py:

  • src/sage/repl/attach.py

    diff --git a/src/sage/repl/attach.py b/src/sage/repl/attach.py
    index c350ec33af..e1c98e32ad 100644
    a b character-by-character:: 
    3636    sage: try:
    3737    ....:     attach(src)
    3838    ....: except Exception:
    39     ....:     traceback.print_exc()
     39    ....:     raise
    4040    Traceback (most recent call last):
    4141    ...
    4242        exec(preparse_file(f.read()) + "\n", globals)
    character-by-character:: 
    4848    sage: try:
    4949    ....:     attach(src)
    5050    ....: except Exception:
    51     ....:     traceback.print_exc()
     51    ....:     raise
    5252    Traceback (most recent call last):
    5353    ...
    5454        exec(code, globals)

comment:9 Changed 2 years ago by jhpalmieri

The following change to repl/attach.py is almost good enough:

  • src/sage/repl/attach.py

    diff --git a/src/sage/repl/attach.py b/src/sage/repl/attach.py
    index c350ec33af..bdfaad4f7f 100644
    a b Check that no file clutter is produced:: 
    2020    ['foobar.sage']
    2121    sage: detach(src)
    2222
    23 In debug mode backtraces contain code snippets. We need to manually
    24 print the traceback because the python doctest module has special
    25 support for exceptions and does not match them
    26 character-by-character::
     23Check printing of code snippets in debug mode::
    2724
    28     sage: import traceback
    2925    sage: with open(src, 'w') as f:
    3026    ....:     _ = f.write('# first line\n')
    3127    ....:     _ = f.write('# second line\n')
    character-by-character:: 
    3329    ....:     _ = f.write('# fourth line\n')
    3430
    3531    sage: load_attach_mode(attach_debug=False)
    36     sage: try:
    37     ....:     attach(src)
    38     ....: except Exception:
    39     ....:     traceback.print_exc()
     32    sage: attach(src)
    4033    Traceback (most recent call last):
    4134    ...
    4235        exec(preparse_file(f.read()) + "\n", globals)
    character-by-character:: 
    4538    sage: detach(src)
    4639
    4740    sage: load_attach_mode(attach_debug=True)
    48     sage: try:
    49     ....:     attach(src)
    50     ....: except Exception:
    51     ....:     traceback.print_exc()
     41    sage: attach(src)
    5242    Traceback (most recent call last):
    5343    ...
    5444        exec(code, globals)

The problem is that it would allow some doctests to pass that should fail. From the file:

    sage: with open(src, 'w') as f:
    ....:     _ = f.write('# first line\n')
    ....:     _ = f.write('# second line\n')
    ....:     _ = f.write('raise ValueError("third")   # this should appear in the source snippet\n')
    ....:     _ = f.write('# fourth line\n')

Then this passes, as it should:

    sage: load_attach_mode(attach_debug=True)
    sage: attach(src)
    Traceback (most recent call last):
    ...
        exec(code, globals)
      File ".../foobar.sage....py", line ..., in <module>
        raise ValueError("third")   # this should appear in the source snippet
    ValueError: third

But this also passes: note that the second to last line is different:

    sage: load_attach_mode(attach_debug=True)
    sage: attach(src)
    Traceback (most recent call last):
    ...
        exec(code, globals)
      File ".../foobar.sage....py", line ..., in <module>
        raise ValueError("third")   # this is gibberish
    ValueError: third

I guess this is what is meant by "We need to manually print the traceback because ..."

comment:10 Changed 2 years ago by chapoton

Nice to see that you are making progress. Sorry that I cannot help.

comment:11 Changed 2 years ago by egourgoulhon

  • Description modified (diff)

comment:12 Changed 2 years ago by egourgoulhon

FWIW, I systematically get these doctest failures outside the patchbot framework, namely on a Ubuntu 18.04 computer when doing make ptestlong, cf. https://groups.google.com/d/msg/sage-release/yFwTkcr5AVY/ifqU7-3ZAAAJ

comment:13 Changed 2 years ago by chapoton

Couldn't some of them be yet another cases of "flush missing", as the previous py3 problems that were fixed ? In the repl_attach case, the correct answer seems to appear in the next doctest.

comment:14 Changed 2 years ago by jhpalmieri

Interesting question. If I add some sys.stdout.flush() commands, then I can get the repl/attach.py doctests to pass when run as in the ticket description (using tee and output redirection), but then doctests fail when run the usual way. I'll keep experimenting.

comment:15 Changed 2 years ago by jhpalmieri

  • Branch set to u/jhpalmieri/flushing

comment:16 follow-up: Changed 2 years ago by jhpalmieri

  • Commit set to 6ade77020fca15a58b5a37eeffdcaf8f2ca1a91c

Okay, here is an attempt at a fix. It fixes the particular problem in the ticket description, at least for me on OS X. Eric, does it help with your situation?

Should I mark it as "needs review", or should some explanatory comments be added? Or is it even the correct approach?


New commits:

6ade770trac 28622: flush or redirect some output

comment:17 Changed 2 years ago by chapoton

This looks good enough for me, even if this is maybe more fixing symptoms than illness.

I will launch my atlas patchbot on the branch.

comment:18 Changed 2 years ago by chapoton

This seems to work with python2 and to fix the python3 issues also on Ubuntu patchbot. I am extremely tempted to give a positive review.

comment:19 in reply to: ↑ 16 ; follow-up: Changed 2 years ago by egourgoulhon

Replying to jhpalmieri:

Okay, here is an attempt at a fix. It fixes the particular problem in the ticket description, at least for me on OS X. Eric, does it help with your situation?

I am just running make ptestlong and shall report soon...

comment:20 in reply to: ↑ 19 Changed 2 years ago by egourgoulhon

Replying to egourgoulhon:

Replying to jhpalmieri:

Okay, here is an attempt at a fix. It fixes the particular problem in the ticket description, at least for me on OS X. Eric, does it help with your situation?

I am just running make ptestlong and shall report soon...

Here is the report from my Ubuntu 18.04 computer:

----------------------------------------------------------------------
sage -t --long --warn-long 53.0 src/sage/rings/polynomial/polynomial_rational_flint.pyx  # 1 doctest failed
----------------------------------------------------------------------
Total time for all tests: 1357.6 seconds

In other words, all the doctests discussed here are passed! The only failure is that discussed in #28334.

So +1 for the positive review.

comment:21 Changed 2 years ago by chapoton

  • Status changed from new to needs_review

ok, so let us first turn to "needs review"

comment:22 Changed 2 years ago by chapoton

John, do you agree that this could be set to positive ?

comment:23 Changed 2 years ago by jhpalmieri

It’s okay with me.

comment:24 Changed 2 years ago by chapoton

  • Authors set to John Palmieri
  • Keywords python3 added
  • Reviewers set to Frédéric Chapoton, ​Eric Gourgoulhon
  • Status changed from needs_review to positive_review

Good. I am setting to positive.

comment:25 follow-up: Changed 2 years ago by dcoudert

Shouldn't we add reference to this ticket to the added lines of code to avoid inadvertent removal ?

comment:26 Changed 2 years ago by git

  • Commit changed from 6ade77020fca15a58b5a37eeffdcaf8f2ca1a91c to 56e1642222b6afd0e35c53458110ee4fc31e7453
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

56e1642trac 28622: add ticket reference when we use stdout.flush()

comment:27 in reply to: ↑ 25 Changed 2 years ago by jhpalmieri

  • Status changed from needs_review to positive_review

Replying to dcoudert:

Shouldn't we add reference to this ticket to the added lines of code to avoid inadvertent removal ?

Done. I'm setting back to positive review, also.

comment:28 Changed 2 years ago by vbraun

  • Branch changed from u/jhpalmieri/flushing to 56e1642222b6afd0e35c53458110ee4fc31e7453
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:29 Changed 16 months ago by mkoeppe

  • Commit 56e1642222b6afd0e35c53458110ee4fc31e7453 deleted
  • Reviewers changed from Frédéric Chapoton, ​Eric Gourgoulhon to Frédéric Chapoton, Eric Gourgoulhon
Note: See TracTickets for help on using tickets.