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.

Sign up for GitHub

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

Jump to bottom

Quantity __array_function__ updates #8884

Merged
merged 5 commits into from
Jun 29, 2019
Merged

Quantity __array_function__ updates #8884

merged 5 commits into from
Jun 29, 2019

Conversation

mhvk
Copy link
Contributor

@mhvk mhvk commented Jun 20, 2019

Note: this should only be merged if the numpy-dev travis run passes.

Fixes recent failures for histogram2d, i0, and block and adds support for piecewise

@mhvk mhvk added units Bug Affects-dev PRs and issues that do not impact an existing Astropy release numpy-dev labels Jun 20, 2019
@mhvk mhvk added this to the v4.0 milestone Jun 20, 2019
@mhvk mhvk requested a review from adrn June 20, 2019 16:03
@mhvk mhvk mentioned this pull request Jun 20, 2019
Closed
11 tasks
@bsipocz
Copy link
Member

bsipocz commented Jun 20, 2019

The only failure on numpy-dev is a fluke, an unrelated url one, I didn't think it was worth restarting.

This was referenced Jun 21, 2019
Closed
Merged
@bsipocz
Copy link
Member

bsipocz commented Jun 28, 2019

Would be nice to get this in to have a green numpy-dev CI job again.

@mhvk
Copy link
Contributor Author

mhvk commented Jun 28, 2019

Yes, would be good to have numpy-dev working again (and move on with the follow-up PRs). It actually is not that tricky a PR... @adrn, @astrofrog?

@bsipocz bsipocz requested a review from astrofrog June 28, 2019 18:51
astrofrog
astrofrog approved these changes Jun 29, 2019
View reviewed changes
Copy link
Member

@astrofrog astrofrog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! A minor non-code comment below regarding making sure we mention the numpy license, but feel free to merge once you've decided whether to do anything about that.

astropy/units/quantity_helper/function_helpers.py
# cannot be used directly. Since it would be very costly to inspect all
# arrays and then turn them back into a nested list, we just copy here the
# second implementation, np.core.shape_base._block_slicing, since it is
# shortest and easiest.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe mention the license for NUMPY is in https://github.com/astropy/astropy/blob/master/licenses/NUMPY_LICENSE.rst (since we are copying code)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe this could go at the top of the file somewhere since this is the case for several functions here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good, point. There are two locations, and I now rewrote the top to read

# Licensed under a 3-clause BSD style license. See LICENSE.rst except
# for parts explicitly labelled as being (largely) copies of numpy
# implementations; for those, see licenses/NUMPY_LICENSE.rst.

@mhvk mhvk merged commit a22f8f9 into astropy:master Jun 29, 2019
@mhvk mhvk deleted the quantity-array-function-updates branch June 29, 2019 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@astrofrog astrofrog astrofrog approved these changes

@adrn adrn Awaiting requested review from adrn

Assignees
No one assigned
Labels
Affects-dev PRs and issues that do not impact an existing Astropy release Bug numpy-dev units
Projects
None yet
Milestone
v4.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mhvk @bsipocz @astrofrog