Skip to content

Quantity updates for numpy dev - #3860

Merged
embray merged 2 commits into
astropy:masterfrom
mhvk:quantity-updates-for-numpy-dev
Jun 30, 2015
Merged

embray merged 2 commits into
astropy:masterfrom
mhvk:quantity-updates-for-numpy-dev

Conversation

@mhvk

@mhvk mhvk commented Jun 17, 2015

Copy link
Copy Markdown
Contributor

WIth numpy/numpy#5964 (for more discussion, see numpy/numpy#5864), a lot of NotImplemented handling is removed from the ufuncs. As a result, np.ufunc does not return NotImplemented any more. This makes much more sense, but brought an inconsistency in our usage to the front, where in Quantity.__mul__ we called np.multiply directly, rather than letting ndarray.__mul__ take care (which can return NotImplemented). This PR corrects that oversight. (I wrote the original myself and am slightly worried there was a reason -- but travis will hopefully tell).

In addition, the above numpy PRs broke the use of a * list, where if a was integer it would be used to make multiple versions of the list. It was actually rather surprising this ever worked (see numpy PR discussion), and so it was decided not to continue to support that in numpy. Since there seems to be no reason really to support that in Quantity either, I changed the tests to more narrowly focus on whether Quantity.__index__() does the right thing.

@mhvk mhvk added units Bug Affects-dev PRs and issues that do not impact an existing Astropy release labels Jun 17, 2015
@mhvk
mhvk force-pushed the quantity-updates-for-numpy-dev branch from e5fdb14 to 82415ef Compare June 17, 2015 23:51
@mhvk

mhvk commented Jun 18, 2015

Copy link
Copy Markdown
Contributor Author

Note: the numpy-dev travis build still fails because of numpy/numpy#5962. I checked locally, though, that with that PR it passes.

@mhvk

mhvk commented Jun 18, 2015

Copy link
Copy Markdown
Contributor Author

@mdboom - could you have a look at this? It now passes also the numpy-dev test, since the above numpy PR was merged. With it, we will be safe for numpy 1.10 at least on python2 (python3 depends on #3854).

@mhvk

mhvk commented Jun 22, 2015

Copy link
Copy Markdown
Contributor Author

@mdboom - it would be helpful for my testing of numpy-dev to have this in. Could you have a quick look? (The appveyor build fail is unrelated; see #3871).

@mhvk mhvk added this to the v1.0.4 milestone Jun 22, 2015
mhvk added 2 commits June 24, 2015 19:04
In numpy-dev, the __index__ can no longer be used to multiply lists;
for discussion about why, really, this was always broken & wrong, see
numpy/numpy#5864.
@mhvk
mhvk force-pushed the quantity-updates-for-numpy-dev branch from 82415ef to c36ce6f Compare June 24, 2015 23:04
@mhvk

mhvk commented Jun 26, 2015

Copy link
Copy Markdown
Contributor Author

With #3871 resolved, appveyor is now happy too. Ready for final review!

embray added a commit that referenced this pull request Jun 30, 2015
@embray
embray merged commit 3fd5262 into astropy:master Jun 30, 2015
@embray

embray commented Jun 30, 2015

Copy link
Copy Markdown
Member

Makes sense, thanks.

@mhvk
mhvk deleted the quantity-updates-for-numpy-dev branch June 30, 2015 20:32
embray added a commit that referenced this pull request Aug 5, 2015
embray added a commit that referenced this pull request Aug 7, 2015
embray added a commit that referenced this pull request Aug 11, 2015
embray added a commit that referenced this pull request Aug 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Affects-dev PRs and issues that do not impact an existing Astropy release Bug units

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants