Using in_ would be the PEP8 appropriate convention if we really want to use the word "in"... but I've always felt that's rather ugly. Maybe one of these?:

• into
• in_unit
• in_units_of

Also, I think once this has been implemented, it would be great to add a basically identical to method to NDData (@wkerzendorf, does this seem like a good idea to you?)

My argument for in_() is that it seems like plain english to me, but of course it could just be me... If I had some value in meters per second, for example, I would say "What is that in kilometers per hour?"

I definitely think the to() method should exist in as many places as possible, and certainly in NDData, because it's very convenient. You could imagine having a more sophisticated one for spectra:

spectrum.to(dispersion=u.angstroms, flux=u.whatever)

or something like that.

I see your point, but my concern is that the (somewhat subtle) distinction between "in" and "to" in English does not make it clear what you mean in code-world. That is "What is that in gallons?" could get an english answer of either "five and a half" or "five and a half gallons". The former (in my mind) would be a float of value 5.5, and the later is a Quantity object. I'm not sure if my suggestions above are any better in that respect, though. What about value_in or value_as or number_as or something like that? Then it's clear from the name that you are asking for the actual number, rather than a new Quantity. And it's still reasonable english to say "What is that number in gallons?" or something like that.

Ah, good point -- I think I like value_in() the best. To me, just reading that makes it clear what is happening:

quantity.value_in(u.meters/u.second)

is a) plain english and b) clear that it's returning just the value in those units. I'll update the API to reflect this and try to solicit for more opinions!

@eteq I'm not quite sure what you're recommending. I think we should have it very similar in NDData and Quantities (as you suggest). At the moment NDData.convert_to does the unit conversion - but we can change that method name.

I added some example code for uncertainties. There are a few points I slipped in there:

• The user should be able to specify any object to the uncertainty argument. If they want to carry an entire probability distribution around, they should be able to!
• We might want to consider writing the uncertainty classes for NDData to be a bit more generic. I think right now they are required to be associated with an NDData object, but for example, you should be able to add two Quantity objects with StandardDeviationUncertainty's and have it do the error propagation for you.
• For most cases, we let the user handle error propagation. This means dropping the uncertainty after performing any operations.

@wkerzendorf - my comment above about the in_ is outdated because adrn adjusted it to value_in (which looks great to me).

Or were you referring to just the to item? I actually didn't realize there was a convert_to in there, but that was exactly what I was thinking. I guess I am indeed suggesting just that either NDData.convert_to be changed to NDData.to or Quantity.to be change to Quantuty.convert_to - the key is consistency. My preference would be to in all cases to match unit.Unit.

An additional question: what does 123 + Quantity(3,'m') do? Does it:

1. raise an exception
2. result in a quantity of 125 m
3. give a float of 126
4. something else?

I think #3. That's the behavior we have right now for Constant objects, which is good because it allows them to be used in places that expect actual numbers. But I am open to other arguments.

Oh, and I mentioned this in an inline comment somewhat tangentially, but you probably want to update this document to make it expressly clear that Quantity always contains a single value, not an array (for arrays w/units and such you use NDData).

@eteq's comment: I think it should be number 1. For it's similar to having a unit less number added to a normal number - which doesn't make sense.

@eteq's previous comment about convert_to and .to: I don't care they should be the same: .to is fine with me.

@wkerzendorf - so you think we should have the behavior of auto-casting to floats that's in Constant be in Constant but not in Quantity? Or you think we should remove that from Constant ? (I strongly disagree with the latter, but am ambivalent about the former)

Let's take a step back as this has wide ranging applications:

• we have quantity which is a number, a unit, and an uncertainty (optional)
• we have nddata which is a collection of numbers, a unit for all those numbers, and an uncertainty (optional) (and some other crap, which we'll ignore for this example (set to None)).

I think both of those things need to work similarly to provide a logical, consistent and easy to learn framework.

I'm going to outline a couple of inconsitencies, for now

In the first example both quantity and nddata have the same units and the same uncertainty type:

• q3 = q1 + q2 which works
• ndd3 = ndd1 + ndd2 which doesn't work, however there's a current workaround ndd3 = ndd1.add(ndd2)

The second example shows how a quantity is instantiated and I guess the same thing should work as well for NDData

• q1 = float(3.) * unit.generic_unit this works and is a mathematically and physically correct construct
  In addition, 3. + unit.generic_unit raises an exception (IMHO rightly so)
• ndd1 = ndarray([...]) * unit.generic_unit this doesn't work yet

The third example involves multiplication and division:

• q2 = float(5) * q1 - works as far as I can tell
• ndd2 = float(5) * ndd1 - doesn't work as of yet

Now the more difficult design decisions and it's implications:

Should we allow addition/subtraction of non-Quantities/NDData objects with Quantities/NDData objects.

Yes:

• q2 = 5 + q1 - what is the type of q2? what unit is q2? That of q1? Does that change for q2 = q1 + 5?
• then we should also allow ndd2 = ndd1 + 5. Same question as above?
  In addition, I guess, we should also allow ndd2 = ndd1 + ndarray([...])

No:

• seemingly no change to Quantities/NDData. However does Constants need to be consistent:
  Currently c1 + 5 is allowed and results in a float (AFAIK)

Choosing the units from the left side of the arithmetic operation vs conversion to common system vs raising an exception:

Left Side:

• One can argue - doesn't matter as unit is kept - slight speed increase
• on the other side - logically addition is commutative and result should not depend on the side
• what happens to implementing addition of non-quantity. Logically, 5 + q1 will behave differently than q1 + 5.

Common system

• addition is commutative
• slightly slower - might cause confusion
• what happens to implementing addition of non-quantity. Logically, 5 + q1 will behave the same as q1 + 5.

Raise Exception

• Cowards way out ;-)
• might be a sensible option for now - as this is a difficult decision.

+1 for 5 + q1 raising an exception. This is basic unit analysis.

Consider this (this is not code):

x = 5 miles
2 + x = 7 miles

You didn't add 2 miles to 5 miles, the statement is "2 plus '5 miles'", and that's meaningless. Further, what does this do?

10 * u.m + 5 + 10 * u.cm

51.1 meters? 10.5 meters? Addition is commutative. This is an error.

@wkerzendorf re: "result should not depend on the side" As I said above, the result is the same whether the resulting units are x or y - the quantity is the same. Regardless of the unit in the object, if you were to use the right or left unit, the two objects would evaluate to being equal. It's clearly an ambiguous situation, but not mathematically so, so whether we default to SI if the units are all SI, CGS if the units are all CGS, SI if there is a mixture, the left unit, the right unit, it's all the same. Any decision is ok. The tutorial would point out that if you want a specific unit, then you convert_to.

This is slightly trickier with NDData. This is valid:

| 1 2 3 |       |  6  7  8 |  
| 4 5 6 | + 5 = |  9 10 11 |
| 7 8 9 |       | 12 13 14 |

One option is to say that if NDData has a unit, only Quantities with compatible units may be added. If they don't, then we can do the operation as above.

Also, why does ndd3 = ndd1 + ndd2 not work, but ndd3 = ndd1.add(ndd2) does? Is this just not implemented yet or is there a reason?

@demitri: I don't know how to quote things yet in the markup - but let me address the different points:

q1 + 5 in my opinion (and yours) should not work as you are adding 5 miles to 2 (what?). So raise exception.
I think to illustrate our example of the necessity of units one should look at: https://www.youtube.com/watch?v=zN9LZ3ojnxY

as for ndd1 + 5 should also not work, however ndd1*5 should work and ndd1 + q1 (with the same units as ndd1) should work as well.

the last comment:
ndd3 = ndd1 + ndd2 was discussed and postponed, but I think it should be implemented (to act as quantities).

I think constants should behave the same: c1 + 5 should raise an exception.

If an NDData object is unitless, adding a scalar to it is mathematically valid...

Just a couple of comments:

• since you bring up the issue of uncertainties again, I'll just point out that this is very hard to do right, and what we have in NDData is not yet a good solution. The difficulty is with correlated errors - as according to normal error propagation rules, a + a (if you treat it as uncorrelated addition) will be different from 2*a. So we need to find a long term solution that works for NDData and Quantity.
• regarding left vs right vs SI vs etc. for operations, subtraction and division are not commutative, so it's not like all cases are ambiguous. Note that it doesn't really matter what the convention is, since as long as it's a quantity after, it will carry some units, and if users want to get the scalar value out, they will explicitly have to convert it to a given unit (e.g. as_value). So maybe it doesn't matter if we pick e.g. the left?

Another way to put this - I had originally thought that Quantity (for scalars and arrays) could live in astropy.units and not ever have to be instantiated directly by the user - for example:

5 * Unit('cm')
np.array([1,2,3]) * Unit('g')

and then we would we leave uncertainties, WCS, masks, and flags for the more complex NDData class, which could always have a ScalarData sub-class. Of course, one could instantiate an NDData from a Quantities object containing an array, but we wouldn't impose all the complexity of NDData on:

np.array([1,2,3]) * Unit('g')

This shouldn't affect the coding of the 0.2 API for Quantity though, but in that case it could just live inside units, as it's just a thin wrapper around units to be able to carry scalars/arrays attached to them.

Agreed that no one should create a Quantity via

q1 = Quantity(12, u.m)

This

x = 5 * u.m

is just really great. That said, I think that the above line should work with only

import astropy
import astropy.unit as u

You can use the Quantity object without even knowing about the Quantity object (but you need units), so one of the two imports above should import it. Maybe that will dictate where it goes.

Second, to get totally pedantic here, can we stop using Unit('cm') even amongst ourselves? We should be pushing the u.cm as the preferred method in tutorials and examples, so we should eat our own dog food.

@demitri - small correction, it will work with just

import astropy.unit as u

i..e there's no need to import the top-level astropy.

@astrofrog Even better. Works for me.

Let me put forward the following suggestion - we implement the current proposed Quantity API as a class inside astropy.units, and this class will never do anything beyond just doing the unit handling. If we want to expand to include uncertainties and meta-data, then we create a class for scalar quantities with errors in astropy.nddata that could inherit for example from Quantity or from NDData (tbd), so that all the uncertainty handling gets done inside astropy.nddata for consistency. Having the base Quantity in astropy.units makes sense, otherwise methods of Unit will need to import from NDData, and it would be nice to avoid circular dependencies. The astropy.units.Quantity class will then be very stable, and we don't have to worry about dealing with the messy issue of uncertainties there (which definitely does not belong in astropy.units). Then we can add examples like:

d = (5 * Unit('m') + 2 * Unit('km')).value_in('cm')

to the astropy.units docs, and have all that code be self-contained inside astropy.units. In fact, if we do this, users will never have to instantiate a Quantity object directly with Quantity(), which is one less class for users to learn.

What @astrofrog is saying sounds good. @adrn are you ok with this scheme? Particularly the aspect where Quantity does not have uncertainties, and instead we put all the uncertainty stuff in NDData? (note that this also leaves a more clear path forward for incorporating NDData into coordinates in 0.3)

(oh, and @astrofrog, your example should be (5 * Unit('m') + 2 * Unit('km')).to('cm').value, because I think we concluded we'd ditch value_in as it's equally simple to do what I'm showing here)

And I agree that with this limited scope it makes sense in astropy.units - I was saying not there if it was a scalar version of NDData. In @astrofrog's plan it definitely belongs in units.

👍 will do.

@astrofrog: I've been wondering about the NDData and Quantity analogy myself: I think there's room for an NDQuantity which NDData inherits from. Should Quantity maybe eat ndarrays?

@wkerzendorf - I don't understand what you're suggesting here. As @astrofrog outlined, Quantity is a quite limited set of functionality. NDData has a different intent (e.g. doesn't necessarily have to have units, does have uncertainties, etc.), so I don't think it makes sense to put them together in a heirarchy. Certainly not now, anyway.

@eteq: He suggested that NDData is not equivalent to an array based Quantity and there might be a missing link between those two.

Anyways, this can be done later and we should get Quantity in first.

@eteq - the .value is fine for me :-) I'd also be in favor (but won't argue too much about this) that it would be easy enough to allow

np.array([1,2,3]) * u.cm

to also give a Quantity class which contains an Numpy array (since Quantity is not related to NDData). Maybe this would be nice too, though could be lower priority for coding.

@astrofrog: I like that.

ok, we can table the discussion of if/how to connect this to NDData. I think we've converged on an answer! 😃

👍

Who's making the Quantity PR? @adrn?

I assume that was the plan, given that he's doing this. But lets first make sure this document is merged before starting on the implementation.

OK, I've updated the document to (hopefully) include the most recent suggestions. If this looks good, merge it, and we'll defer any more discussion until I have a pull request for the actual code (soon).

Nice work -- 125 comments on this one!

That's > 1.2 comments per line! ;-) Looks good to me, though as I said above, I'm pretty sure the uncertainty stuff shouldn't go in the base Quantity class that will live in astropy.units. You could consider moving any uncertainty related stuff to the end of the document, but it doesn't really matter.

I agree with @astrofrog that it would make sense to move the uncertainty stuff to the end and add a note like "Quantity may end up not even having uncertainties, depending on what we decide in 0.3 for how to divide NDData and Quantity functionality".

Other than that, looks great, @adrn!

Alrighty!

@eteq @astrofrog I think we've come to an agreement -- can we merge this pull request? I've started working on the code, so I'd like to discourage people from adding any more discussion here, and rather comment on the code pull request when that happens. Thanks!

Looks good to me!

Looks good to me as well, and it looks like everyone else too as best as I can tell. So I'll go ahead and merge it.

Thanks for working on this, @adrn - for the implementation you're doing now, you're not going to include uncertainties, right?