Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#17837 closed enhancement (fixed)

Remove matrix_modn_dense.pyx

Reported by: Jeroen Demeyer Owned by:
Priority: major Milestone: sage-6.6
Component: linear algebra Keywords:
Cc: Merged in:
Authors: Jeroen Demeyer Reviewers: Vincent Delecroix
Report Upstream: N/A Work issues:
Branch: 766f1a5 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Jeroen Demeyer)

This has been unused since #4260, remove it.

Change History (18)

comment:1 Changed 8 years ago by Jeroen Demeyer

Authors: André Apitzsch

comment:2 Changed 8 years ago by Jeroen Demeyer

Branch: u/jdemeyer/ticket/17837
Created: Feb 23, 2015, 11:23:05 AMFeb 23, 2015, 11:23:05 AM
Modified: Feb 23, 2015, 11:23:43 AMFeb 23, 2015, 11:23:43 AM

comment:3 Changed 8 years ago by Jeroen Demeyer

Commit: e7681685dc628352e7f0b8d63bffd8fb059897aa
Status: newneeds_review

Do we even need the determinant method in the first place? Why not rely on inheritance?


New commits:

e768168fix some cython warnings

comment:4 Changed 8 years ago by Jeroen Demeyer

Status: needs_reviewneeds_work

comment:5 Changed 8 years ago by Jeroen Demeyer

Description: modified (diff)

comment:6 Changed 8 years ago by Jeroen Demeyer

Authors: André ApitzschJeroen Demeyer
Description: modified (diff)
Summary: Remove redundancies from matrix_modn_dense.pyxRemove matrix_modn_dense.pyx

comment:7 Changed 8 years ago by git

Commit: e7681685dc628352e7f0b8d63bffd8fb059897aa74d2c66c1955fc7b73e24e0203686e95398ee501

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

74d2c66Remove matrix_modn_dense

comment:8 Changed 8 years ago by git

Commit: 74d2c66c1955fc7b73e24e0203686e95398ee501a6112cec279b21c9ae6db3d4a912986693d699de

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

a6112ceRemove matrix_modn_dense

comment:9 Changed 8 years ago by git

Commit: a6112cec279b21c9ae6db3d4a912986693d699de766f1a5a00018c9317cead9dd588d91f17676c68

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

766f1a5Remove matrix_modn_dense

comment:10 Changed 8 years ago by Jeroen Demeyer

Status: needs_workneeds_review

comment:11 Changed 8 years ago by Vincent Delecroix

Reviewers: Vincent Delecroix
Status: needs_reviewpositive_review

Instead of

        if not (isinstance(b, Matrix_modn_dense_float) or
                isinstance(b, Matrix_modn_dense_double)):

you might have used

        if not isinstance(b, (Matrix_modn_dense_float, Matrix_modn_dense_double)):

which is handled in the exact same way by cython.

I will try to use the nice unpickle_override for #17824.

comment:12 Changed 8 years ago by Vincent Delecroix

Status: positive_reviewneeds_work

Overlaps with #10734.

comment:13 Changed 8 years ago by Volker Braun

Branch: u/jdemeyer/ticket/17837766f1a5a00018c9317cead9dd588d91f17676c68
Resolution: fixed
Status: needs_workclosed

comment:14 Changed 8 years ago by Vincent Delecroix

Commit: 766f1a5a00018c9317cead9dd588d91f17676c68

What is this transition "needs work -> closed"!?

comment:15 Changed 8 years ago by Jeroen Demeyer

On the other hand, I don't understand why the mere existence of ticket #10734 would be a reason for needs_work.

comment:16 in reply to:  11 Changed 8 years ago by Jeroen Demeyer

Replying to vdelecroix:

Instead of

        if not (isinstance(b, Matrix_modn_dense_float) or
                isinstance(b, Matrix_modn_dense_double)):

you might have used

        if not isinstance(b, (Matrix_modn_dense_float, Matrix_modn_dense_double)):

which is handled in the exact same way by cython.

Obviously, I just didn't think of that. It doesn't really matter anyway...

comment:17 in reply to:  15 ; Changed 8 years ago by Vincent Delecroix

Replying to jdemeyer:

On the other hand, I don't understand why the mere existence of ticket #10734 would be a reason for needs_work.

Because there is a large amount of work on ticket #10734 that uses matrix_modn_dense. Before throwing away this work, it would have been nice to wait for the current status of this work. I am pretty sure that everything can be achieved with the LinBox classes anyway.

comment:18 in reply to:  17 Changed 8 years ago by Jeroen Demeyer

Replying to vdelecroix:

Because there is a large amount of work on ticket #10734 that uses matrix_modn_dense.

But with or without #17837, matrix_modn_dense is deprecated and no longer used. So if #10734 really uses Matrix_modn_dense, it should be changed anyway.

Note: See TracTickets for help on using tickets.