Opened 4 years ago

Closed 4 years ago

#24863 closed defect (fixed)

Fix signature of Matrix_gfpn_dense.__init__

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-8.2
Component: linear algebra Keywords:
Cc: SimonKing Merged in:
Authors: Jeroen Demeyer Reviewers: Simon King
Report Upstream: N/A Work issues:
Branch: 3c079b2 (Commits, GitHub, GitLab) Commit: 3c079b23ac97481f067a07799089207cc3b2bb44
Dependencies: Stopgaps:

Status badges

Description

The signature of Matrix_gfpn_dense.__init__ is different from the other matrix constructors. Fix this.

Change History (9)

comment:1 Changed 4 years ago by jdemeyer

  • Branch set to u/jdemeyer/fix_signature_of_matrix_gfpn_dense___init__

comment:2 follow-up: Changed 4 years ago by jdemeyer

  • Commit set to 3c079b23ac97481f067a07799089207cc3b2bb44
  • Status changed from new to needs_review

Simon: this is really part of #24742 but I made a separate ticket in case the particular __init__ signature is somehow important to you.


New commits:

3c079b2Fix signature of Matrix_gfpn_dense.__init__

comment:3 in reply to: ↑ 2 Changed 4 years ago by SimonKing

Replying to jdemeyer:

Simon: this is really part of #24742 but I made a separate ticket in case the particular __init__ signature is somehow important to you.

No, the commonly used signature is fine for me.

Last edited 4 years ago by SimonKing (previous) (diff)

comment:4 Changed 4 years ago by jdemeyer

So, would you be willing to give positive review (assuming that tests pass)?

comment:5 follow-up: Changed 4 years ago by jdemeyer

Note that the mutability parameter is non-standard, but I actually like it.

comment:6 in reply to: ↑ 5 ; follow-up: Changed 4 years ago by SimonKing

Replying to jdemeyer:

Note that the mutability parameter is non-standard, but I actually like it.

I am not sure if I actually use that parameter, but if possible I'd prefer it to be preserved.

The patchbot complains about a non-ascii character. Apart from that, it would be positive review.

comment:7 in reply to: ↑ 6 Changed 4 years ago by jdemeyer

  • Reviewers set to Simon King
  • Status changed from needs_review to positive_review

Replying to SimonKing:

The patchbot complains about a non-ascii character.

I don't think so. It looks like the patchbot plugin itself is generating a UnicodeDecodeError.

comment:8 Changed 4 years ago by jdemeyer

Note that the code of Matrix_gfpn_dense.__init__ will change completely in #24742, but those should make no functional difference.

comment:9 Changed 4 years ago by vbraun

  • Branch changed from u/jdemeyer/fix_signature_of_matrix_gfpn_dense___init__ to 3c079b23ac97481f067a07799089207cc3b2bb44
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.