Skip to content

Conversation

@taldcroft
Copy link
Member

This is needed for #3011. Before I write tests etc will this idea and implementation be acceptable?

@taldcroft taldcroft added this to the v1.0.0 milestone Oct 22, 2014
@taldcroft taldcroft self-assigned this Oct 22, 2014
@mhvk
Copy link
Contributor

mhvk commented Oct 22, 2014

@taldcroft - not sure this is the way to go for #3011 (will comment there about that), but in principle I don't mind the idea of an insert method. But for Quantity specifically, I would make it as close as possible to np.insert, and indeed use that function, which could be as short as:

def insert(self, obj, values, axis=None):
    out_array = np.insert(self.value, obj, self._to_own_unit(values), axis)
    return self._new_view(out_array)

p.s. EDITED to correct above. It does work:

from astropy.units import Quantity; from astropy.time import TimeDelta
def insert(self, obj, values, axis=None):
    out_array = np.insert(self.value, obj, self._to_own_unit(values), axis)
    return self._new_view(out_array)

q = u.Quantity([2., 3., 4.], 's')
insert(q, 1, 5*u.ms)
# -> <Quantity [ 2.   , 0.005, 3.   , 4.   ] s>

@taldcroft
Copy link
Member Author

D'oohh! Yes, that is much better, thanks.

@taldcroft
Copy link
Member Author

This was driven by thinking about getting add_row working for #3011. I'm interested to hear any suggestions before I tackle this.

@taldcroft
Copy link
Member Author

@mhvk - implemented your idea and added tests. This is ready for final review (assuming that travis passes).

Copy link
Contributor

Choose a reason for hiding this comment

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

Not a big deal, but it may be good to give an example where one sees the unit conversion is done. So perhaps insert 10*u.km?

@mhvk
Copy link
Contributor

mhvk commented Oct 23, 2014

Apart from my small comment, this looks good. Indeed, better than good -- nice to see the test that checks that conversion of float to int leads to an exception!

@taldcroft
Copy link
Member Author

@mhvk - it turns out there are more issues related to older versions of numpy and integer Quantities. In particular:

>>> q = u.Quantity([1, 2], unit='m', dtype=int)
>>> q2 = q.to(u.km)
>>> q2.dtype
dtype('float64')

I think this is reasonable behavior because in general unit conversions can turn things into float. But this made it kind of silly/annoying to write tests that start from integer quantities.

@mhvk
Copy link
Contributor

mhvk commented Oct 23, 2014

@taldcroft - the build failure seems fake -- I restarted it.

@taldcroft
Copy link
Member Author

Me too, maybe at the exact same moment...

taldcroft added a commit that referenced this pull request Oct 23, 2014
@taldcroft taldcroft merged commit c4784e7 into astropy:master Oct 23, 2014
@taldcroft taldcroft deleted the quantity-insert branch October 23, 2014 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants