Skip to content

Division of Quantity by a scalar wrong #679

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
Merged
embray merged 2 commits into from
Jan 25, 2013
Merged

Division of Quantity by a scalar wrong #679

embray merged 2 commits into from
Jan 25, 2013

Conversation

@eteq
Copy link
Member

@eteq eteq commented Jan 25, 2013 

In [1]: from astropy.units import Quantity as Q

In [2]: Q(1, 'm/s') / 2
Out[2]: <Quantity 2.0 s / (m)>

This is appears to be doing the inverse of what it should be. FWIW the actual inverse operation is correct:

In [3]: 2 / Q(1, 'm/s')
Out[3]: <Quantity 2.0 s / (m)>

This just seems to affect division by scalars. Division by other quantities (and curiously, even numpy scalars arrays) is correct:

In [12]: Q(1, 'm/s') / np.array(2)
Out[12]: <Quantity 0.5 m / (s)>

@embray
Copy link
Member Author

embray commented Jan 25, 2013

FWIW, it looks like the code for Quantity.__div__ that's causing this used to be in Quantity.__rdiv__ but was moved into __div__ as part of 24d45e2 for reasons that are unclear to me.

@embray embray mentioned this pull request Jan 25, 2013
Merged
@eteq
Copy link
Member

eteq commented Jan 25, 2013

Definitely a bug. The "reason" was that __rdiv__ was fixed first, and the code was copied and pasted over, swapping the division order. Apparently I missed that one elif clause. It turns out that isn't necessary anyway, though, because the power of the unit is the same in __div__, unlike __rdiv__ where it's flipped.

I also added a regression test. Feel free to merge and backport immediately if you like it, @iguananaut

@embray
Copy link
Member Author

embray commented Jan 25, 2013

Cool thanks--this is exactly all I did to fix it.

embray added a commit that referenced this pull request Jan 25, 2013
@embray
Merge pull request #679 from eteq/issue-679
3bbd36f 
Division of Quantity by a scalar wrong
@embray embray merged commit 3bbd36f into astropy:master Jan 25, 2013
embray added a commit that referenced this pull request Jan 25, 2013
@embray
Merge pull request #679 from eteq/issue-679
e449a4e 
Division of Quantity by a scalar wrong
keflavich pushed a commit to keflavich/astropy that referenced this pull request Oct 9, 2013
@embray
Merge pull request astropy#679 from eteq/issue-679
98593cf 
Division of Quantity by a scalar wrong
@eteq eteq deleted the issue-679 branch May 3, 2015 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Bug units

Projects

None yet

Milestone

v0.2.0

Development

Successfully merging this pull request may close these issues.

2 participants

@eteq @embray