Skip to content
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.

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

Quantity constructor changes logarithmic units #6319

Open
bwinkel opened this issue Jul 4, 2017 · 3 comments
Open

Quantity constructor changes logarithmic units #6319

bwinkel opened this issue Jul 4, 2017 · 3 comments

Comments

@bwinkel
Copy link
Contributor

bwinkel commented Jul 4, 2017

I like the astropy units module a lot. Thanks for this great piece of work!

I think, I encountered a bug related to the Quantities class. When I use logarithmic units, the constructor seems to change the type of the unit. This does not happen, when I simply multiply with the unit:

# astropy.__version__ == '3.0.dev19502'

import numpy as np
from astropy import units as u
from astropy.units import Quantity

# define units
dimless = u.dimensionless_unscaled
dB = u.dB(dimless)

arr = np.array([1, 3, 5])

q1 = (arr * dB)         # q1.unit == Unit("dB(1)")
q2 = Quantity(arr, dB)  # q2.unit == Unit("dB")

type(q1.unit)  # astropy.units.function.logarithmic.DecibelUnit
type(q2.unit)  # astropy.units.function.mixin.RegularFunctionUnit

This is a problem for some of my use cases:

q1.to(dimless)  # <Quantity [ 1.25892541, 1.99526231, 3.16227766]>
q2.to(dimless)  # UnitConversionError: 'dB' and '' (dimensionless) are not
                # convertible

@u.quantity_input(a=dB)
def my_func(a):
    print(a)

my_func(q1)  # [ 1.  3.  5.] dB
my_func(q2)  # UnitsError: Argument 'a' to function 'my_func' must be in
             # units convertible to 'dB'.

I can work-around the first problem:

u.add_enabled_equivalencies(u.logarithmic())

q1.to(dimless)  # <Quantity [ 1.25892541, 1.99526231, 3.16227766]>
q2.to(dimless)  # <Quantity [ 1.25892541, 1.99526231, 3.16227766]>

But now the decorated function fails in both cases (with a different error!)

my_func(q1)  # ValueError: Unit  is not a physical unit.
my_func(q2)  # ValueError: Unit  is not a physical unit.
@mhvk
Copy link
Contributor

mhvk commented Jul 4, 2017

Hmm, this is similar to #5851, where the question was why Quantity constructor did not return a function quantity such as Magnitude when passed a magnitude unit. As discussed there, in general, one wants to have a class initializer return an instance of that class. So, here, the initializer returns a Quantity with units of u.dB since that is the same thing as u.dB(u.one) - it fails it you do u.Quantity(1., u.dB(u.mW)). Following #5851, we decided that we would at least allow this to work if one passed in subok=True (this works since u.Dex is a subclass of Quantity). So, I think that answers part of your question - the main question here is whether we could perhaps document this better? (though it is in the help for Quantity in the latest version)

However, you also raise a new issue here, that quantity_input does not work in your example. I do think this would have to be a=u.dB(u.dimensionless_unscaled), but that doesn't work either. It should be possible to make this work, perhaps with subok=True used by default, or by allowing to request a specific quantity subclass. cc @adrn, @Cadair who wrote quantity_input.

@bwinkel
Copy link
Contributor Author

bwinkel commented Jul 5, 2017

Thanks a lot. Using subok=True indeed fixes most of my problems, except the one with quantity_input. Perhaps, as you say, the subok=True should be made default? I'm developing a new package and therefore, not only astropy docs need to clarify this, but I'd also need to do it in my docs, which would add a lot of overhead.

@mhvk
Copy link
Contributor

mhvk commented Apr 13, 2018

@Cadair - as you have just been looking at quantity_input, what do you think of using subok=True by default in the Quantity constructor, so that things work for logarithmic units as well? (this issue)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants