Skip to content

Conversation

@mhvk
Copy link
Contributor

@mhvk mhvk commented Oct 13, 2013

I noticed the following weird behaviour:

import astropy.units as u; import numpy as np
u.Quantity(2, 'm') * ['a', 'b', 'c']

yields

['a', 'b', 'c', 'a', 'b', 'c']

So, the integer in the Quantity is taken as a repeat counter. Apparently, after Quantity appropriately fails to produce a multiplication, the list's __rmul__ manages to access the array without checking the unit --- certainly int(u.Quantity(2, 'm') fails appropriately.

@astrofrog
Copy link
Member

This might not be something we can do anything about, but it's good to know the problem exists.

@embray
Copy link
Member

embray commented Oct 11, 2013

That's bizarre. I had always assumed that the iterable times integer operation in Python was non-commutative. But clearly I was wrong in that assumption.

@mhvk
Copy link
Contributor Author

mhvk commented Oct 13, 2013

@astrofrog, @embray - found the culprit: we needed to define __index__, which is used to convert numpy integers to python indices. For quantities, this should only happen if they are dimensionless unscaled.

@astrofrog
Copy link
Member

@mhvk - thanks for identifying this! It would be nice to include the original issue you reported here as a regression test.

@mhvk
Copy link
Contributor Author

mhvk commented Oct 13, 2013

@astrofrog - OK, better test added.

@mdboom
Copy link
Contributor

mdboom commented Oct 14, 2013

👍

astrofrog added a commit that referenced this pull request Oct 14, 2013
Integer quantity times list should not be treated as repitition count
@astrofrog astrofrog merged commit f4b62d6 into astropy:master Oct 14, 2013
@mhvk mhvk deleted the quantity-define-index branch October 14, 2013 16:47
@embray
Copy link
Member

embray commented Oct 14, 2013

Wow--a Python feature I did not know existed. Looks like it was added specifically for Numpy...

mhvk added a commit to mhvk/astropy that referenced this pull request Oct 15, 2013
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.

4 participants