Opened 5 years ago

Closed 5 years ago

#24947 closed defect (fixed)

meataxe doctest failure on 8.2.beta8

Reported by: Vincent Delecroix Owned by:
Priority: blocker Milestone: sage-8.2
Component: linear algebra Keywords:
Cc: Jeroen Demeyer, Simon King Merged in:
Authors: Jeroen Demeyer Reviewers: Simon King
Report Upstream: N/A Work issues:
Branch: 447b4e5 (Commits, GitHub, GitLab) Commit: 447b4e567170715e234a13d5d4695c21292e034e
Dependencies: Stopgaps:

Status badges

Description

Mainly because the constructor of matrices changed

      File "sage/matrix/matrix_gfpn_dense.pyx", line 304, in sage.matrix.matrix_gfpn_dense.Matrix_gfpn_dense.__init__ (build/cythonized/sage/matrix/matrix_gfpn_dense.c:4524)
        def __init__(self, parent, entries=None, *, bint copy=False, bint coerce=False, bint mutable=True):
    TypeError: __init__() takes at most 2 positional arguments (4 given)

See quasar short log

Change History (13)

comment:1 Changed 5 years ago by Vincent Delecroix

Summary: meataxe failing on 8.2.beta8meataxe doctest failure on 8.2.beta8

comment:2 Changed 5 years ago by Simon King

I guess that's the change in matrix initialisation introduced by Jeroen. But hasn't that been tested with optional matrix backends?

comment:3 in reply to:  2 ; Changed 5 years ago by Vincent Delecroix

Replying to SimonKing:

I guess that's the change in matrix initialisation introduced by Jeroen. But hasn't that been tested with optional matrix backends?

I just did :-) One problem is that on the previous beta, there was a constant failure on patchbots (#24918). Hence the patchbot quasar (running with all optional packages) was off. Until everybody agrees that each beta must pass with all optional packages you will have to wait (see how much do we support optional packages and #23832).

Last edited 5 years ago by Vincent Delecroix (previous) (diff)

comment:4 Changed 5 years ago by Jeroen Demeyer

Sorry for that...

This is actually fixed in #24742. Should I create a separate fix for just this issue or does anybody plan to review #24742?

comment:5 in reply to:  4 Changed 5 years ago by Vincent Delecroix

Replying to jdemeyer:

Sorry for that...

This is actually fixed in #24742. Should I create a separate fix for just this issue or does anybody plan to review #24742?

Better to have a fix in an independent ticket (this one). #24742 is likely to take more time.

comment:6 Changed 5 years ago by Jeroen Demeyer

Authors: Jeroen Demeyer
Component: packages: optionallinear algebra

comment:7 Changed 5 years ago by Jeroen Demeyer

Branch: u/jdemeyer/meataxe_doctest_failure_on_8_2_beta8

comment:8 in reply to:  3 Changed 5 years ago by Vincent Delecroix

Commit: 447b4e567170715e234a13d5d4695c21292e034e

Replying to vdelecroix:

Replying to SimonKing:

I guess that's the change in matrix initialisation introduced by Jeroen. But hasn't that been tested with optional matrix backends?

I just did :-) One problem is that on the previous beta, there was a constant failure on patchbots (#24918). Hence the patchbot quasar (running with all optional packages) was off. Until everybody agrees that each beta must pass with all optional packages you will have to wait (see how much do we support optional packages and #23832).

Actually #24918 is not merged in the new beta so that the patchbots are still useless.


New commits:

447b4e5Properly fix signature of Matrix_gfpn_dense.__init__

comment:9 Changed 5 years ago by Jeroen Demeyer

Status: newneeds_review

comment:10 Changed 5 years ago by Jeroen Demeyer

Interestingly, this failure is caused by the combination of two merged tickets, each of which individually passed the testsuite. This is just to say that even a perfect patchbot cannot catch everything.

comment:11 Changed 5 years ago by Jeroen Demeyer

ping

comment:12 Changed 5 years ago by Simon King

Reviewers: Simon King
Status: needs_reviewpositive_review

If I understand correctly, you added your commit on 11th of March, but the patchbot gets errors on 12th of March.

However, both tests failures that I see are in test(interacts.statistics.coin), which apparently is unrelated with this ticket. And the commit looks good to me.

comment:13 Changed 5 years ago by Volker Braun

Branch: u/jdemeyer/meataxe_doctest_failure_on_8_2_beta8447b4e567170715e234a13d5d4695c21292e034e
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.