Skip to content

Conversation

@mhvk
Copy link
Contributor

@mhvk mhvk commented Mar 23, 2015

EDIT -- now rewritten such that one just does u.mag(u.ct/u.s), where u.mag is a standard unit that has a __call__ method.

ORIGINAL TEXT:

Just as a proof of concept, this allows one to do, e.g.,

import astropy.units as u
from astropy.units.function import mag, dex, dB
mag(u.ct/u.s)
# MagUnit('ct / s')

To be useful, it would imply using the function unit mag as the default instance for u.mag. This would require more work.

@embray
Copy link
Member

embray commented Mar 24, 2015

I haven't looked at the code, but I like the concept!

@mhvk
Copy link
Contributor Author

mhvk commented Jun 8, 2015

Note to self: if we do this, u.mag should almost certainly refer to units.function.mag rather than units.astrophys.mag -- the latter would just be used internally in the function units (or in composite units).

@mhvk mhvk added this to the v1.1.0 milestone Jun 8, 2015
@mhvk
Copy link
Contributor Author

mhvk commented Jun 8, 2015

Set the 1.10 milestone, since I think with this merged, the preferred API for function units changes: one would do u.dex(u.cm/u.s**2) instead of u.DexUnit(u.cm/u.s**2). And if this is still done in the same milestone, we don't have to make any explicit notices about preferred API.

@mhvk mhvk force-pushed the units-callable branch 2 times, most recently from 3a50d38 to 442a18a Compare June 10, 2015 16:54
@mhvk
Copy link
Contributor Author

mhvk commented Jun 10, 2015

@mdboom - here's an update on the callable units. It now makes mag, dex, and dB be subclass instances of their normal classes which have an additional __call__ method. Can you have a look that you think the implementation is OK? In principle, one could also add a __call__ method to UnitBase (which normally would raise a TypeError), but I thought it was better to keep the changes completely separate. It does mean that mag, dex and dB are now defined in function.units instead of in astrophys, but that seems, if anything, a more logical place (and in practice one would just get them from the top level anyway).

I'll try to update the documentation later today.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think reusing the same name here is a bit confusing (when used in the function/units.py file below). Maybe call it IrreducibleFunctionUnit. Same for FunctionUnit below...

@mdboom
Copy link
Contributor

mdboom commented Jun 10, 2015

Looks good other than my one minor comment.

@mhvk
Copy link
Contributor Author

mhvk commented Jun 10, 2015

@mdboom - OK, changed those names. I now also updated the documentation, including giving a little teaser in the "getting started" section of units.

@mhvk mhvk changed the title WIP Callable mag, dex, dB units Callable mag, dex, dB units Jun 10, 2015
@mhvk mhvk changed the title Callable mag, dex, dB units Callable mag, dex, dB units, and parsing of such units. Jun 10, 2015
@mhvk mhvk force-pushed the units-callable branch 2 times, most recently from 5b9dc67 to 6c4a71b Compare June 10, 2015 23:46
@mhvk mhvk mentioned this pull request Jun 11, 2015
@mhvk mhvk force-pushed the units-callable branch from 6c4a71b to c0e9dcf Compare June 11, 2015 16:52
Since Quantity.__div__ already calls __truediv__ instead of the other
way around.
@mhvk mhvk force-pushed the units-callable branch from f35b225 to 2de1938 Compare June 11, 2015 18:32
@mhvk
Copy link
Contributor Author

mhvk commented Jun 12, 2015

@mdboom - I think this is now all done (I now use the callable units in the parser and resolved a small issue found while testing with __numpy_ufunc__); if you want to check anything, perhaps the revised documentation?

@mhvk
Copy link
Contributor Author

mhvk commented Jun 17, 2015

@mdboom - would you want/have a chance to have a last look at this PR? (And the follow-up in #3851...)

mdboom added a commit that referenced this pull request Jun 17, 2015
Callable mag, dex, dB units, and parsing of such units.
@mdboom mdboom merged commit a06aed7 into astropy:master Jun 17, 2015
@mhvk mhvk deleted the units-callable branch June 17, 2015 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants