Opened 12 years ago

Closed 16 months ago

#5110 closed enhancement (duplicate)

rewrite diagonal_matrix to be more general

Reported by: jason Owned by: was
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: linear algebra Keywords:
Cc: Merged in:
Authors: Reviewers: Samuel Lelièvre, Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

See the patches at #3704, which rewrite diagonal_matrix. On that ticket, however, we finally just went with a basic patch that fixed the specific bug mentioned. There is good code in the other patches that ought to be used, though. This ticket is here so that we eventually use the cleaner rewrite contained in the first patches at #3704.

Change History (10)

comment:1 Changed 8 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:2 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:3 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:4 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:5 Changed 3 years ago by gh-black-stones

  • Report Upstream set to N/A

What exactly should be added to diagonal_matrix? I'd be happy to work on this ticket but it looks like the other patches in #3704 add doctests.

comment:6 Changed 3 years ago by kcrisman

I think the idea is to use attachment:trac-3704-diagonal_matrix.patch:ticket:3704 to make a more generally applicable function.

That said, there are probably more important tickets to work on, notably reviewing many in symbolics ...

comment:7 Changed 3 years ago by gh-black-stones

I did see that patch, but looking at the source code for diagonal_matrix(), the differences seem to be

  1. Changing the parameters
  2. More comments and doctests
  3. Casting to Sequence

1 could be a good fix since the current function uses "arg0, arg1, arg2" which is pretty bad, but in my opinion, 2 and 3 are not necessary (although if we do change 1 we will need to change 2).

comment:8 Changed 16 months ago by slelievre

  • Milestone changed from sage-6.4 to sage-duplicate/invalid/wontfix
  • Reviewers set to Samuel Lelièvre
  • Status changed from new to needs_review

This is probably solved by #10604 (please check).

comment:9 Changed 16 months ago by tscrim

  • Status changed from needs_review to positive_review

I think so as well.

comment:10 Changed 16 months ago by slelievre

  • Resolution set to duplicate
  • Reviewers changed from Samuel Lelièvre to Samuel Lelièvre, Travis Scrimshaw
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.