Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#17837 closed enhancement (fixed)

Remove matrix_modn_dense.pyx

Reported by: jdemeyer 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 jdemeyer)

This has been unused since #4260, remove it.

Change History (18)

comment:1 Changed 7 years ago by jdemeyer

  • Authors set to André Apitzsch

comment:2 Changed 7 years ago by jdemeyer

  • Branch set to u/jdemeyer/ticket/17837
  • Created changed from 02/23/15 11:23:05 to 02/23/15 11:23:05
  • Modified changed from 02/23/15 11:23:43 to 02/23/15 11:23:43

comment:3 Changed 7 years ago by jdemeyer

  • Commit set to e7681685dc628352e7f0b8d63bffd8fb059897aa
  • Status changed from new to needs_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 7 years ago by jdemeyer

  • Status changed from needs_review to needs_work

comment:5 Changed 7 years ago by jdemeyer

  • Description modified (diff)

comment:6 Changed 7 years ago by jdemeyer

  • Authors changed from André Apitzsch to Jeroen Demeyer
  • Description modified (diff)
  • Summary changed from Remove redundancies from matrix_modn_dense.pyx to Remove matrix_modn_dense.pyx

comment:7 Changed 7 years ago by git

  • Commit changed from e7681685dc628352e7f0b8d63bffd8fb059897aa to 74d2c66c1955fc7b73e24e0203686e95398ee501

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

74d2c66Remove matrix_modn_dense

comment:8 Changed 7 years ago by git

  • Commit changed from 74d2c66c1955fc7b73e24e0203686e95398ee501 to a6112cec279b21c9ae6db3d4a912986693d699de

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

a6112ceRemove matrix_modn_dense

comment:9 Changed 7 years ago by git

  • Commit changed from a6112cec279b21c9ae6db3d4a912986693d699de to 766f1a5a00018c9317cead9dd588d91f17676c68

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

766f1a5Remove matrix_modn_dense

comment:10 Changed 7 years ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:11 follow-up: Changed 7 years ago by vdelecroix

  • Reviewers set to Vincent Delecroix
  • Status changed from needs_review to positive_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 7 years ago by vdelecroix

  • Status changed from positive_review to needs_work

Overlaps with #10734.

comment:13 Changed 7 years ago by vbraun

  • Branch changed from u/jdemeyer/ticket/17837 to 766f1a5a00018c9317cead9dd588d91f17676c68
  • Resolution set to fixed
  • Status changed from needs_work to closed

comment:14 Changed 7 years ago by vdelecroix

  • Commit 766f1a5a00018c9317cead9dd588d91f17676c68 deleted

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

comment:15 follow-up: Changed 7 years ago by jdemeyer

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 7 years ago by jdemeyer

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 ; follow-up: Changed 7 years ago by vdelecroix

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 7 years ago by jdemeyer

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.