matrix vstack/hstack can fail with immutable matrix as first argument

[cbm755]

>>> A = Matrix([[1, 2], [pi, 4]])
>>> AIm = sympify(A)

>>> type(A)
sympy.matrices.dense.MutableDenseMatrix

>>> type(AIm)
sympy.matrices.immutable.ImmutableMatrix

>>> Matrix.vstack(A, AIm)
⎡1  2⎤
⎢    ⎥
⎢π  4⎥
⎢    ⎥
⎢1  2⎥
⎢    ⎥
⎣π  4⎦

>>> Matrix.vstack(AIm, A)
('hello vstack, cls=', <class 'sympy.matrices.dense.MutableDenseMatrix'>)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-37-aec335eeafd0> in <module>()
----> 1 Matrix.vstack(AIm, A)

/home/cbm/work/octsympy/octsympy.git/inst/sympy/matrices/matrices.py in vstack(cls, *args)
   4541         """
-> 4542         return reduce(cls.col_join, args)
   4543 
   4544     def xreplace(self, rule):  # should mirror core.basic.xreplace

TypeError: unbound method col_join() must be called with MutableDenseMatrix instance as first argument (got ImmutableMatrix instance instead)

Note it will work if one uses MatrixBase.vstack.

I don't think this is really about immutable matrices; its about cls.col_join not being the right method, depending on what is in args.

One way to fix this would be to take cls from the type of args[0] instead:

kls = type(args[0])
reduce(kls.col_join, args)

Although I'm not sure that's quite right.

Another option is to avoid reduce and use:

M = args.pop(0)
for i = range(len(args)):
    M = M.col_join(args.pop(0))

(or more pythonic version thereof).

Whatever we do hstack needs the same fix.

[Abdullahjavednesar]

I want to try this.

[Abdullahjavednesar]

@cbm755 this the output in my system and seems to be fine.

In [36]: A=Matrix([[1,2],[pi,4]])

In [37]: AIm=sympify(A)

In [38]: type(A)
Out[38]: sympy.matrices.dense.MutableDenseMatrix

In [39]: type(AIm)
Out[39]: sympy.matrices.immutable.ImmutableMatrix

In [40]: Matrix.vstack(AIm,A)
Out[40]:
?1  2?
?    ?
?π  4?
?    ?
?1  2?
?    ?
?π  4?

In [41]: Matrix.hstack(AIm,A)
Out[41]:
?1  2  1  2?
?          ?
?π  4  π  4?

[cbm755]

Thanks for checking. I've tried it and I can reproduce with Python 2 on both Sympy 1.0.1 and the current git master. But on Python 3, it works as you show: nice catch.

[cbm755]

I want to try this.

Go for it. It should be an easy fix, I'm just not enough of a Python expert to know which fix is the best. Please add a test and read the https://github.com/sympy/sympy/wiki/Development-workflow.

[mamakancha]

Hi @Abdullahjavednesar . Let me know in case you're not working on it :)

[siefkenj]

This is fixed in #11592 , for what it's worth :-)

[cbm755]

@siefkenj that's great! I wondered when I saw your refactor but I haven't had a chance to look yet. Thanks.

@Abdullahjavednesar if that PR goes in first, we'll need to update your PR #11951 to just add tests---this issue will still need tests even if its fixed elsewhere.