Converting a constant to a Quantity breaks __repr__

[mhvk]

import astropy.units as u

import astropy.constants

u.Quantity(astropy.constants.c)
/usr/lib/python2.7/site-packages/IPython/core/formatters.py:239: FormatterWarning: Exception in text/latex formatter: 'unicode' object has no attribute '_repr_latex_'
  FormatterWarning,
Out[3]: <repr(<astropy.units.quantity.Quantity at 0x7fba9bb288c0>) failed: AttributeError: 'Quantity' object has no '_unitstr' member>

[mhvk]

The problem here (and perhaps also with #3538) is that Constant misses __array_finalize__ and hence does not update itself properly when being viewed as a new object:

import astropy.constants as c
c.c.view(c.Constant)
# <repr(<astropy.constants.constant.Constant at 0x7fc60bf6f828>) failed: AttributeError: 'Constant' object has no 'name' member>

[mhvk]

OK, the now attached fixes to Constant should do it. @mdboom - please have a look, as I had to change the internals of Constant somewhat.

[astrofrog]

ping @embray since he wrote a lot of the constants code

[mdboom]

So we are privatizing a public property here?

[mhvk]

No, not quite so bad: since Constant subclasses from Quantity, it still has the unit property (which just returns _unit, i.e., this thing here). I had to make sure, though, that _unit was a Unit instance, which is what this lazyproperty takes care of.

[mdboom]

Ah, I see. Makes sense, then.

[embray]

I'm not entirely sure I follow though. Why do we need the property at all if this is also handled by Quantity now?

[mhvk]

Maybe I am the one who misunderstood: Constants are intialised with units passed in as a string. My impression was that this unit could not be parsed immediately since some units depend on constants. Is this true? I thought this was the reason they were passed through Unit only later, using lazyproperty. The change I made was to rename the string-based unit from _unit to _unit_string, so that both _unit and unit can return Unit objects (since it was a problem for Quantity if _unit was a string rather than a Unit instance).

[mhvk]

Just to explain: part of the logic of changing _unit to _unit_string is that some Quantity internals assume that its subclasses have a proper Unit object in their _unit attribute (this is partially for speed-up, so one doesn't have to pass the unit through the Unit initializer all the time).

[embray]

Just so I'm clear here, is the default here going to be equivalent to subok=True or subok=False? I don't see the point of converting a Constant down to a Quantity where unnecessary, since it's only dropping additional information for no gain.

[mhvk]

@embray - if one passes through u.Quantity(<constant>), one will by default get a Quantity (i.e., subok=False). I also do not see a particular point of doing that, but it seems a sufficiently rare case that it is not worth adding an if-statement that would slow things down for other uses. And at least it should not raise any errors!

[embray]

Okay. I guess that's fine--most of the time if someone where doing u.Quantity it would be just to convert/check that everything passed in to some routine is a Quantity to use in a calculation. In most cases that means combining it with other quantities/values in which case maintaining the Constant-ness is meaningless.

[mhvk]

@embray - indeed, the issue was triggered by assert_quantity_allclose failing on u.Quantity(<constant>) -- and there it is fine if the constant-ness is lost.
But is my PR right? In particular, is it true that the unit has to be stored as a string initially because initialising the actual Unit would cause an import loop?

[embray]

Sounds like for the most part there's no further issue here so I'll go ahead and merge.