LinearCode.basis() should be an immutable Sequence

[nathanncohen]

Right now the .basis() method returns just a list. This should instead be an immutable Sequence, retaining the type information in its .universe() method.

CC: @sagetrac-dlucas

Component: coding theory

Author: Johan Rosenkilde, Nathann Cohen

Branch: b3dc9bf

Reviewer: David Lucas, Daniel Augot

Issue created by migration from https://trac.sagemath.org/ticket/19251

[nathanncohen]

Branch: public/19251

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

f49c204

trac #19251: A real matrix as LinearCode.basis()

[sagetrac-git]

Commit: f49c204

[sagetrac-dlucas]

comment:3

Hello,

Just a question: if you're not adding a deprecation warning to this (which I agree with), what do you think about adding extra info to the docstring, like
"Returns a basis of self as a matrix of the ambient space of self"?

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

6cc755d

trac #19251: Better description

[sagetrac-git]

Changed commit from f49c204 to 6cc755d

[nathanncohen]

comment:5

Here it is!

Nathann

[sagetrac-dlucas]

Reviewer: David Lucas

[sagetrac-dlucas]

comment:6

Great!

As all tests pass, I'm giving the green bulb.

David

[nathanncohen]

comment:7

Thanks!

[vbraun]

comment:8

Doctests fail.

Also, a matrix is not a basis. Matrices have row and column spans, its not clear which one you mean if you just return a matrix. This is how it is supposed to work:

sage: (QQ^3).span([(1,2,3)]).basis()
[
(1, 2, 3)
]
sage: _.universe()
Vector space of degree 3 and dimension 1 over Rational Field
Basis matrix:
[1 2 3]

[nathanncohen]

comment:9

Never discuss with a release manager.

Nathann

[johanrosenkilde]

comment:10

There's still something of a bug, though, since the type returned by LinearCode.basis() is list, while what Volker writes about is a Sequence_generic, which exactly supports the universe method.

[vbraun]

comment:11

+1 to returning an immutable Sequence instead of a list... are you going to implement that?

[sagetrac-git]

Changed commit from 6cc755d to 91114c5

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

7e0416d	Revert basis() returning a matrix to returning a list
91114c5	LinearCode.basis() returns a Sequence

[kevindilks]

comment:15

One failing test in documentation due to formatting.

File "src/doc/en/prep/Quickstarts/Graphs-and-Discrete.rst", line 325, in doc.en.prep.Quickstarts.Graphs-and-Discrete
Failed example:
    D.basis()
Expected:
    [(1, 0, 1, 0, 1, 0, 1), (0, 1, 1, 0, 0, 1, 1), (0, 0, 0, 1, 1, 1, 1)]
Got:
    [
    (1, 0, 1, 0, 1, 0, 1),
    (0, 1, 1, 0, 0, 1, 1),
    (0, 0, 0, 1, 1, 1, 1)
    ]

[sagetrac-git]

Changed commit from 91114c5 to 69fb5c4

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

9ccbb3e	Merge remote-tracking branch 'trac/develop' into t/19251/public/19251
69fb5c4	Fixed doctest in documentation text

[johanrosenkilde]

comment:17

Missed that one - thanks. Rebased and doctest fixed.

[sagetrac-git]

Changed commit from 69fb5c4 to b3dc9bf

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

b3dc9bf

fixed conflict

[sagetrac-danielaugot]

comment:20

Hi

this is perfectly immutable in my session. Nevertheless, I add to resolve a conflict when pulling.

I set it to positive review

Daniel

[johanrosenkilde]

Changed reviewer from David Lucas to David Lucas, Daniel Augot

[vbraun]

Changed branch from public/19251 to b3dc9bf

[jdemeyer]

Changed commit from b3dc9bf to none

[jdemeyer]

Changed author from Johan S. R. Nielsen, Nathann Cohen to Johan Rosenkilde, Nathann Cohen