Repository navigation
Cosmology units - #1237
Cosmology units#1237
Conversation
There was a problem hiding this comment.
You could replace this line with
self._hubble_distance = (1 / self._H0).to(u.Mpc)
And then remove c_kms completely.
There was a problem hiding this comment.
That doesn't work -- not surprisingly, since H0 is inverse time units, not distance -- unless there has been some change to units since I branched so that it automatically inserts factors of c as needed.
H0 = 70 * u.km / u.s / u.Mpc
(1/H0).to(u.Mpc)
ERROR: UnitsException: 'Mpc s / km' (time) and 'Mpc' (length) are not convertible [astropy.units.core]
There was a problem hiding this comment.
Sorry, my mistake. I should have written
self._hubble_distance = (const.c / self._H0).to(u.Mpc)
which should then work, and you can still remove c_kms.
|
Apart from the comments above, this looks great to me. |
There was a problem hiding this comment.
I don't know if we do this now or as a follow-on, but we should probably consider making Distance a Quantity subclass (as we have done for Angle in #1006). This would give make most of the properties that convert the value to other units redundant (i.e. they could be removed), as well as make conversions much easier.
There was a problem hiding this comment.
Probably a good idea... but in a different pull request.
|
Cool -- this is probably the most major use of the Thanks -- these are all mostly minor comments. We probably can (and should) put off inheriting |
|
Ok, all done except the Distance subclassing, which I think belongs elsewhere. distmod does return a u.mag Quantity rather than just a floating point number (or ndarray). As discussed elsewhere (#406) this isn't very useful right now, but, hey, maybe at some future date it will be. |
* Distances, etc. now returned as astropy.units.Quantity objects * Some cosmological variables (Tcmb0, H0, etc.) stored as quantities * Unit tests updated to reflect use of quantities. * coordinates.Distance makes use of cosmology object, so those uses updated.
* Following suggestion by @nhmc, use constants.c directly instead of c_kms * For consistency, distmod also returns a quantity object.
* Mostly minor documentation and line break changes. * use Quantity(blah, u.Mpc) instead of Quantity(blah, 'Mpc')
|
@astrofrog -- done. It passes all the cosmology related tests on python 2.7 and 3.2. |
|
Can you file a new issue for the breakage on 3.2? Travis is testing master on 3.2 just fine at the moment, so I suspect there is some environmental difference going on... EDIT: request -> issue |
|
@mdboom: I was just in the process of doing that -- but now I can't seem to replicate it. Maybe some cruft left over from a previous build? Anyways, ignore that. |
Cosmology units
Address issue #406 by adding units to astropy.cosmology. Passes all unit tests on python3.2. Unfortunately, I don't have a working sphinx installation, so I haven't been able to test if the doc update worked.