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

Quiver crashes if given matrices #1546

Closed
wants to merge 1 commit into from
Closed

Quiver crashes if given matrices #1546

wants to merge 1 commit into from

Conversation

gustavla
Copy link

As it is now, quiver() fails if you give it numpy.matrix objects, as this simple example shows:

>>> quiver(np.matrix(np.ones((32, 32))), np.matrix(np.ones((32, 32))))

It does so because it runs ravel() on the input, which if the input is a matrix gives a row matrix of shape (N, 1), instead of an array of shape (N,), which is what quiver expects in further calculations.

This pull request fixes this problem by running the input through np.asarray() before ravel() is called.

@WeatherGod
Copy link
Member

It might have to be asanyarray(). I don't know how quiver
currently behaves with masked arrays.

@efiring
Copy link
Member

efiring commented Nov 30, 2012

On 2012/11/29 6:00 PM, Benjamin Root wrote:

It might have to be asanyarray(). I don't know how quiver
currently behaves with masked arrays.

From the docstring:

U, V, C may be masked arrays, but masked X, Y are not
supported at present.

Using asanyarray() would not help with a matrix because it is a subclass
of ndarray.

The proposed changeset will break support for masked arrays, and is
therefore unacceptable.

My inclination is to declare that matplotlib does not support the matrix
subclass, period. If you work with matrices, convert them to ndarrays
before using them as arguments to mpl functions. The only general
alternative would be for mpl to put in special matrix checking all over
the place, and I really don't want to see that. I would be amazed if
quiver is the only place where a matrix input causes trouble.

@gustavla
Copy link
Author

I see, yeah that's a problem. The function np.ma.asarray looked promising here, but it actually just converts a matrix into a masked matrix.

It's hard to realize what the problem is if you happen to do this though, so it would be great if we could spare the people who accidentally do this some headache, in any way possible. Should I just open up an issue?

@efiring
Copy link
Member

efiring commented Nov 30, 2012

You could open an issue. Unless other developers think we should put in extensive matrix trapping, the solution would be to put clear "matrix inputs are not supported and may cause hard-to-diagnose errors" warnings in the documentation.

Thank you for pointing out the problem and proposing a solution, but I am going to close this pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants