Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

matrix() command should accept some Matlab-style inputs #11699

Open
dandrake opened this issue Aug 18, 2011 · 33 comments
Open

matrix() command should accept some Matlab-style inputs #11699

dandrake opened this issue Aug 18, 2011 · 33 comments

Comments

@dandrake
Copy link
Contributor

In https://groups.google.com/d/msg/sage-devel/p-nrpKUBMm8/LUMAfoXsz-UJ Jason Grout mentioned that it would be convenient if Sage accepted the following Matlab syntax for creating matrices:

sage: matrix("1 2 3; 4 5 6; 7 8 9")
[1 2 3]
[4 5 6]
[7 8 9]

This should be an easy change to the matrix() command, and would be very convenient for users, even those with no Matlab background.

Depends on #24742

CC: @kini @kcrisman @mforets

Component: linear algebra

Author: Dan Drake

Branch/Commit: u/ddrake/string-matrix-11699 @ 5d006ef

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

@jasongrout
Copy link
Member

@jasongrout
Copy link
Member

comment:2

In particular, elements can be separated by either commas or spaces (or both), and rows are separated by semicolons. So we should be able to easily separate by splitting the string:

rows = a.split(';')
for r in rows:
    elts = r.replace(',', ' ').split()
    # do something to interpret the elts, like map(sage_eval, elts)

@jasongrout
Copy link
Member

comment:3

Or probably even faster, do the ','->' ' translation once at the beginning to avoid lots of small strings being created:

# if a was the string passed in as the list of elements
a=a.replace(',', ' ')
rows = a.split(';')
elements=[map(sage_eval, r.split()) for r in rows]
# now continue matrix processing with "elements" as the list of elements

This would just be inserted in the first time the elements of a matrix were accessed and would convert the string to a list of elements like it expects.

@jasongrout
Copy link
Member

comment:4

In fact, we should support multiple lines here too, like Matlab, so you could do:

matrix("""
1 2 3
4 5 6
7 8 9
""")

To do that, do

# if a was the string passed in as the list of elements
a=a.replace(',', ' ').replace(';','\n')
rows = a.split('\n')
# throw away empty rows
rows = [r for r in rows if not r.trim()]
elements=[map(sage_eval, r.split()) for r in rows]
# now continue matrix processing with "elements" as the list of elements

At this point, it might be better to use regular expressions. It's certainly worth a timing test. Note the following matlab examples:

>> a=[1 2 3
4 5 6]

a =

     1     2     3
     4     5     6

>> a=[
1 2 3
4 5 6
]

a =

     1     2     3
     4     5     6

>> a=[  
1 2 3;
4 4 4
]

a =

     1     2     3
     4     4     4

>> a = [
1 2 3; ;
4 5 6]

a =

     1     2     3
     4     5     6

>> a = [;;1 2;;3 4] 

a =

     1     2
     3     4

>> a=[

;
1 2 3]

a =

     1     2     3



@jasongrout
Copy link
Member

comment:5

Oops, that should r.strip() rather than r.trim() above.

@jasongrout
Copy link
Member

comment:6

Slightly faster than the double replace, if s is defined at module creation time:

import string
s=string.maketrans(';,','\n ')

Then the double replace above becomes: a.translate(s)

@jasongrout
Copy link
Member

comment:7

Okay, here's a new iteration, especially since I flipped logic above with the r.strip()

# if a was the string passed in as the list of elements
a=a.replace(',', ' ').replace(';','\n') # or a.translate(s) if we can initialize s
rows = a.split('\n')
# throw away empty rows
rows = [r for r in rows if r.strip()]
elements=[map(sage_eval, r.split()) for r in rows]
# now continue matrix processing with "elements" as the list of elements

@dandrake
Copy link
Contributor Author

comment:8

Okay, try this. I think it still needs some optimization with the string handling stuff, and it could probably use some more doctests, but it's basically ready for review.

@dandrake
Copy link
Contributor Author

Author: Dan Drake

@dandrake

This comment has been minimized.

@dandrake
Copy link
Contributor Author

comment:9

Hrm, Robert Bradshaw just tossed some cold water on my simple string-handling: https://groups.google.com/d/msg/sage-devel/p-nrpKUBMm8/_4DL5li7NcQJ

The quick version: what about 1 + sin(x) (for splitting in spaces) and [1, 2; pi, f(3, 4)] for splitting on commas? Hrm.

@jasongrout
Copy link
Member

comment:10

quick comment: you should probably use isinstance(args[0], basestring), as that is forward-compatible with unicode strings and python3.

@dandrake
Copy link
Contributor Author

comment:11

Another problem: sage_eval evaluates its arguments in a different context than the one in which matrix() gets called. So even though you may have done x = var('x'), it doesn't know about x.

If we do put this in, I think the docstring should be explicit that using strings is limited, and only really works for number literals and similar things. Then later someone can write a full parser and figure out the evaluation context stuff.

@jasongrout
Copy link
Member

comment:13

Maybe on another ticket, but can I ask for one more enhancement on this? Can we make the following work:


matrix("""
[1 2]
[3 4]
""")

(i.e., any "]" or "[" next to row delimiters are stripped). That would mean that I could cut and paste output from Sage back into Sage to make a matrix.

@dandrake
Copy link
Contributor Author

comment:14

Replying to @jasongrout:

Maybe on another ticket, but can I ask for one more enhancement on this? Can we make the following work:


matrix("""
[1 2]
[3 4]
""")

(i.e., any "]" or "[" next to row delimiters are stripped). That would mean that I could cut and paste output from Sage back into Sage to make a matrix.

Sure, no problem. Although you'll still have the split-on-whitespace and context limitations. But plain numerical matrices are common enough so that I think it's worth it.

@dandrake
Copy link
Contributor Author

apply to main repo

@jasongrout
Copy link
Member

comment:15

Attachment: trac_11699_matlab_input_to_matrix.patch.gz

See #12354 for another take on this as preparser syntax.

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Apr 6, 2012

comment:17

This patch causes a doctest to fail in sageinspect.py (see patchbot logs) -- looks like somebody helpfully wrote a doctest which relies on the docstring of "matrix()" being exactly 34 lines long. (There is also another failure on the most recent run, but I suspect that is just a random glitch.)

@loefflerd loefflerd mannequin added s: needs work and removed s: needs review labels Apr 6, 2012
@jasongrout
Copy link
Member

comment:18

argh, I was trying to use this today, thinking it was already in...

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@dandrake
Copy link
Contributor Author

Commit: ddc2e81

@dandrake
Copy link
Contributor Author

Branch: u/ddrake/string-matrix-11699

@dandrake
Copy link
Contributor Author

Changed work issues from failing doctest to failing doc build

@dandrake
Copy link
Contributor Author

comment:24

I've ported this over to 6.9.beta3. It builds, works, and passes doctests, but the documentation doesn't build -- I have doctests with \n in them, and these are not interpreted by Sphinx correctly, and then it complains about indentation being messed up. Advice for how to fix that welcome.


New commits:

ddc2e81add Matlab-style strings for matrix() constructor

@jdemeyer
Copy link

comment:25

Replying to @dandrake:

Advice for how to fix that welcome.

Use

r"""
docstring
"""

instead of

"""
docstring
"""

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 27, 2015

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

5d006efraw string for docstrings so Sphinx handles \n -- thanks Jeroen Demeyer

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 27, 2015

Changed commit from ddc2e81 to 5d006ef

@dandrake
Copy link
Contributor Author

comment:27

Replying to @jdemeyer:

Use

r"""
docstring
"""

instead of

"""
docstring
"""

That's it! Thank you.

@dandrake
Copy link
Contributor Author

Changed work issues from failing doc build to none

@videlec
Copy link
Contributor

videlec commented Oct 21, 2015

comment:29

Failing doctest

sage -t --long src/sage/misc/sageinspect.py
**********************************************************************
File "src/sage/misc/sageinspect.py", line 1959, in sage.misc.sageinspect.sage_getsourcelines
Failed example:
    sage_getsourcelines(matrix)[1]
Expected:
    732
Got:
    867
**********************************************************************
1 item had failures:
   1 of  28 in sage.misc.sageinspect.sage_getsourcelines
    [284 tests, 1 failure, 28.10 s]
----------------------------------------------------------------------
sage -t --long src/sage/misc/sageinspect.py  # 1 doctest failed
----------------------------------------------------------------------

@jdemeyer
Copy link

jdemeyer commented Mar 1, 2018

Dependencies: #24742

@jdemeyer
Copy link

jdemeyer commented Mar 1, 2018

comment:32

I don't plan to work on this, but if anybody does: it should be on top of #24742.

@jdemeyer

This comment has been minimized.

@mkoeppe mkoeppe removed this from the sage-6.4 milestone Dec 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants