BUG: Let linspace accept input that has an array_interface. #6659
Conversation
…rwise a numpy or python type.
Package-Manager: portage-2.2.20.1
|
This would need a test. Otherwise, LGTM. |
|
That sounds a good idea but I am not sure how to generate something that would break otherwise without involving |
|
In [ |
| start = start * 1. | ||
| stop = stop * 1. | ||
| # Make sure one can use variables that have an __array_interface__, gh-6634 | ||
| from numpy import asarray |
There was a problem hiding this comment.
Please use asanyarray here, so that numpy subclasses get passed through (np.linspace works with Quanity right now; this would break it). Note also that you can just add this import to the ones imported from numeric at the top.
Actually, the multiplication with 1. is not necessary anymore if you just do
start = asanyarray(start, dtype=np.float64)
There was a problem hiding this comment.
No, dtype=float will break complex.
Please do add a test for a simple array subclass though... @mhvk can you contribute one?
There was a problem hiding this comment.
Arg, yes, of course, should have thought about complex numbers. But, actually, why bother with the multiplication at all? Below, float(num) is entered into result_type, so it cannot be integer anyway.
There was a problem hiding this comment.
for inspiration for tests, see ma/tests/test_subclassing.py
There was a problem hiding this comment.
I think there are some special cases that would break otherwise such as single return value. But there should be tests for that, so go ahead and try ;).
|
See my comment about |
|
Thank you for all those contributions and comments while I was having my daily sleep, I will use |
|
Is this a regression in 1.10 and we should try to get it into 1.10.2? Still needs a test! |
|
I know! I have been a little bit overworked the last few days (more like the last 2 weeks), and I have to learn a few things before I am even able to write a test. |
|
I could work on a test for this. Just to be clear, was the issue in #6634 that the sage type does not coerce to float when right-multiplied by a float? |
|
In essence yes, although I don't remember checking left multiplication. And as you can see it attracted a small wish list. |
|
@kiwifb I'm not sure what you mean by "it attracted a small wish list"? |
|
(Left multiplication isn't an issue here either--all the test really needs is a dummy class that behaves similarly to the sage type in this regard, and check that it works as start stop (and step?) for linspace. |
|
By wishlist, I mean people have asked for specific tests for other cases. My initial patch was using |
|
Ah ok--the |
|
☔ The latest upstream changes (presumably #7268) made this pull request unmergeable. Please resolve the merge conflicts. |
|
This looks simple enough to finish up if someone wants to take it on. |
|
I currently don't have time or the knowledge to write the requested tests case. Erik Bray was going to look at it but some other stuff is keeping him busy. |
|
@kiwifb If you'll just rebase this PR I can make a PR to your PR to add a test. |
|
OK will do in the next 24 hours. |
|
Rebased. Your turn @embray! |
|
Added a regression test at kiwifb#1 for @kiwifb to merge in. One problem I ran into in writing this test is that |
|
Hmmm, yeah, seems like a bug. It appears that the empty tuple is supported if |
Adds a regression test that demonstrates the issue.
|
OK it looks like this is finally ready to be merged! |
|
So can this be merged now? |
|
Yes, I am not really wanting to invite more criticism because it would delay it further ;) |
|
Just note that imho this wasn't a numpy bug. The function clearly asks for a scalar, so that it doesn't work with something that is neither a scalar nor an array scalar is not surprising. I'd expect multiple functions in numpy and scipy to fail if they get passed Sage reals. |
Needed to support sagemath which passes non-ndarray arguments that have array_interface methods.
Fixes i#6634.