{"action":"created","issue":{"url":"https://api.github.com/repos/jobovy/galpy/issues/258","repository_url":"https://api.github.com/repos/jobovy/galpy","labels_url":"https://api.github.com/repos/jobovy/galpy/issues/258/labels{/name}","comments_url":"https://api.github.com/repos/jobovy/galpy/issues/258/comments","events_url":"https://api.github.com/repos/jobovy/galpy/issues/258/events","html_url":"https://github.com/jobovy/galpy/pull/258","id":124695038,"number":258,"title":"Add support for units using the astropy Units framework","user":{"login":"jobovy","id":1044876,"avatar_url":"https://avatars.githubusercontent.com/u/1044876?v=3","gravatar_id":"","url":"https://api.github.com/users/jobovy","html_url":"https://github.com/jobovy","followers_url":"https://api.github.com/users/jobovy/followers","following_url":"https://api.github.com/users/jobovy/following{/other_user}","gists_url":"https://api.github.com/users/jobovy/gists{/gist_id}","starred_url":"https://api.github.com/users/jobovy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jobovy/subscriptions","organizations_url":"https://api.github.com/users/jobovy/orgs","repos_url":"https://api.github.com/users/jobovy/repos","events_url":"https://api.github.com/users/jobovy/events{/privacy}","received_events_url":"https://api.github.com/users/jobovy/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/jobovy/galpy/labels/enhancement","name":"enhancement","color":"0b02e1"}],"state":"open","locked":false,"assignee":{"login":"jobovy","id":1044876,"avatar_url":"https://avatars.githubusercontent.com/u/1044876?v=3","gravatar_id":"","url":"https://api.github.com/users/jobovy","html_url":"https://github.com/jobovy","followers_url":"https://api.github.com/users/jobovy/followers","following_url":"https://api.github.com/users/jobovy/following{/other_user}","gists_url":"https://api.github.com/users/jobovy/gists{/gist_id}","starred_url":"https://api.github.com/users/jobovy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jobovy/subscriptions","organizations_url":"https://api.github.com/users/jobovy/orgs","repos_url":"https://api.github.com/users/jobovy/repos","events_url":"https://api.github.com/users/jobovy/events{/privacy}","received_events_url":"https://api.github.com/users/jobovy/received_events","type":"User","site_admin":false},"milestone":{"url":"https://api.github.com/repos/jobovy/galpy/milestones/11","html_url":"https://github.com/jobovy/galpy/milestones/v1.2","labels_url":"https://api.github.com/repos/jobovy/galpy/milestones/11/labels","id":1036885,"number":11,"title":"v1.2","description":"","creator":{"login":"jobovy","id":1044876,"avatar_url":"https://avatars.githubusercontent.com/u/1044876?v=3","gravatar_id":"","url":"https://api.github.com/users/jobovy","html_url":"https://github.com/jobovy","followers_url":"https://api.github.com/users/jobovy/followers","following_url":"https://api.github.com/users/jobovy/following{/other_user}","gists_url":"https://api.github.com/users/jobovy/gists{/gist_id}","starred_url":"https://api.github.com/users/jobovy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jobovy/subscriptions","organizations_url":"https://api.github.com/users/jobovy/orgs","repos_url":"https://api.github.com/users/jobovy/repos","events_url":"https://api.github.com/users/jobovy/events{/privacy}","received_events_url":"https://api.github.com/users/jobovy/received_events","type":"User","site_admin":false},"open_issues":4,"closed_issues":19,"state":"open","created_at":"2015-03-24T18:13:11Z","updated_at":"2016-04-20T19:38:04Z","due_on":"2016-06-30T04:00:00Z","closed_at":null},"comments":0,"created_at":"2016-01-04T04:32:42Z","updated_at":"2016-04-22T21:42:51Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/jobovy/galpy/pulls/258","html_url":"https://github.com/jobovy/galpy/pull/258","diff_url":"https://github.com/jobovy/galpy/pull/258.diff","patch_url":"https://github.com/jobovy/galpy/pull/258.patch"},"body":"This pull request will add the following:\r\n - [x] Support for returning Quantities with appropriate units for Orbit instances:\r\n   - [x] For those methods using physical_conversion\r\n   - [x] For other methods (e.g., ``orbit.ra``)\r\n   - [x] Action-angle angle coordinates?\r\n - [x] Return orbit position as a SkyCoord\r\n - [x] Support for providing the input phase-space position using Quantities\r\n - [x] Allowing integration times to be specified using Quantities\r\n - [x] Support for time arguments of functions like ``orbit.ra`` to be specified using Quantities\r\n - [ ] Support *any* input to *any* Orbit method as Quantity:\r\n   - [x] ro and vo at setup\r\n   - [x] ro and vo for methods using physical_conversion\r\n   - [x] Still need to do ``obs=`` keyword for almost all methods (in ``_parse_radec_kwargs``)\r\n   - [ ] orbit fit? ``vxvv`` and ``vxvv_err`` inputs\r\n   - [x] delta and b for actionAngle? Needs to be done as part of actionAngle re-write (bc just passed to actionAngle setup)\r\n - [x] Configuration ``ro`` and ``vo`` as Quantity\r\n - [x] ``ro=`` and ``vo=`` scales at Potential initialization for all potentials\r\n   - [x] Set to default if parameters are given with units\r\n   - [x] Method and function to turn physical output on / off\r\n - [x] Support for initializing Potentials with mass, scales, etc. as Quantities; needs to be done for every potential individually (because no simple definition of the amplitude applies to all potentials), but write general conversion wrapper that potential-implementers can choose from (saying what the amplitude means: density, mass, velocity^2, whatever else we currently have).\r\n   - [x] General parsing of amp w/ units as part of Potential\r\n   - [x] What about the power-law spherical and same w/ cut-off potentials?\r\n   - [x] Need to parse scale and other parameters in each individual potential\r\n   - [x] Need to check that amp units are correct for each potential\r\n   - [x] FlattenedPowerPotential needs scale\r\n   - [x] What are the units for Kuzmin-Kutuzov? Mass\r\n   - [x] Need to make amp for PowerSphericalPotential mass for consistency with Kepler\r\n   - [x] Should we implement this for 2D and 1D potentials? Or just Error?\r\n   - [x] Support for users who don't use G=1.\r\n - [x] Support for returning Potential methods' output using Quantities (forces etc.)\r\n - [x] Support for allowing the inputs to Potential methods that are functions of ``R,z,phi=,t=`` to be Quantities; any general way or does every Potential need to be re-written?\r\n  - [x] Potential\r\n  - [x] planarPotential and linearPotential\r\n - [x]  Bypass the previous wrapper for orbit integration internally, as it has significant overhead\r\n - [x]  Bypass the previous wrapper for action/angle evaluation internally, as it has significant overhead\r\n - [x] Support *any* input to *any* Potential method as Quantity: ``mvir`` (skip), ~~``plot``~~, ~~``plotDensity``~~, ~~``lindbladR``~~, ~~``rl``~~, ~~``vterm``~~, ~~``toVertical``~~, ~~``plotRotcurve``~~, ~~``plotEscapecurve``~~, ``conc`` (skip), ~~``plotPotentials``~~, ~~``plotDensities``~~, ~~``rl(Pot,lz)``~~, ~~``lindbladR(Pot,OmegaP)``~~, ~~``vterm(Pot,l)``~~, ~~``RZToVerticalPotential``~~\r\n - [x] Plot rotation and escape curve on axes with physical units when relevant\r\n - [x] Interpolated potential should inherit ``ro`` and ``vo``\r\n - [x] Should we also return actions etc. in ``actionAngle`` using Quantities? Straightforward, but perhaps tedious?\r\n  - [x] Re-write general ``actionAngle`` setup to just handle new ``ro`` and ``vo`` below; write general ``parse_args`` function to do what the initialization does currently\r\n  - [x] Re-write functions so they all use common ``__call__``, ``actionsFreqs``, ``actionFreqsAngles``? This should get units\r\n  - [x] Re-write setup such that they run the general actionAngle setup that sets ``ro=`` and ``vo=``\r\n  - [x] What about the non-general functions? Tempted to leave these alone\r\n  - [x] Other setup inputs with units (e.g., ``b`` or ``delta``, ``tintJ``)\r\n  - [x] Inputs to methods that can have units (e.g., ``ts`` for actionAngleIsochroneApprox)\r\n  - [x] Do the estimateDelta or estimateb functions return proper physical outputs for potentials that have units?\r\n  - [x] Inputs to methods can have units\r\n  - [x] Check that the units of given potential are the same as those of the ``actionAngle`` object\r\n  - [x] Check that the units of a given orbit are the same as those of the ``actionAngle`` object?\r\n  - [x] Need to check that orbit interface uses unit-free version\r\n  - [x] Need to make sure that qDF still works\r\n  - [x] Need to make sure that streamdf and streamgapdf still work\r\n - [ ] DFs using Quantities\r\n   - [x] Setup needs to handle ``ro`` and ``vo`` below; make general?\r\n   - [x] Hide vmomentsurfacendensity and vmomentdensity everywhere (prepend ``_``)\r\n   - [x] Inputs of diskdf\r\n   - [x] Outputs of diskdf\r\n   - [x] Sample of diskdf\r\n   - [x] Setup of diskdf\r\n   - [x] Inputs of evolveddiskdf\r\n   - [x] Outputs of evolveddiskdf\r\n   - [x] Inputs of quasiisothermaldf\r\n   - [x] Outputs of quasiisothermaldf\r\n   - [x] Setup of quasiisothermaldf\r\n   - [x] Sample of quasiisothermaldf\r\n   - [x] Inputs to streamdf\r\n   - [ ] Outputs of streamdf\r\n   - [x] Setup of streamdf\r\n   - [ ] Sample of streamdf\r\n   - [ ] Inputs to streamgapdf\r\n   - [ ] Outputs of streamgapdf\r\n   - [ ] Setup of streamgapdf\r\n   - [ ] Sample of streamgapdf\r\n - [ ] Give MWPotential2014 units?\r\n\r\nRequires tests for:\r\n - [x] Support for returning Quantities with appropriate units for Orbit instances:\r\n   - [x] For those methods using physical_conversion\r\n   - [x] For other methods (e.g., ``orbit.ra``)\r\n   - [x] Action-angle angle coordinates?\r\n - [x] Return orbit position as a SkyCoord\r\n - [x] Support for providing the input phase-space position using Quantities\r\n - [x] Allowing integration times to be specified using Quantities\r\n - [x] Support for time arguments of functions like ``orbit.ra`` to be specified using Quantities\r\n - [ ] Support *any* input to *any* Orbit method as Quantity:\r\n   - [x] ro and vo at setup\r\n   - [x] ro and vo for methods using physical_conversion\r\n   - [x] ``obs=`` keyword for almost all methods (in ``_parse_radec_kwargs``)\r\n   - [ ] orbit fit?\r\n   - [ ] delta and b for actionAngle?\r\n - [x] Configuration ``ro`` and ``vo`` as Quantity\r\n - [x] ``ro=`` and ``vo=`` scales at Potential initialization for all potentials\r\n    - [x] Turning physical output on/off for instances and lists\r\n - [x] Support for initializing Potentials with mass, scales, etc. as Quantities\r\n   - [x] General parsing of amp w/ units as part of Potential\r\n   - [x] Need to parse scale and other parameters in each individual potential\r\n   - [x] Need to check that amp units are correct for each potential\r\n   - [x] Also for 2D and 1D potentials\r\n - [x] Support for returning Potential methods' output using Quantities (forces etc.)\r\n - [x] Support for allowing the inputs to Potential methods to be Quantities; any general way or does every Potential need to be re-written?\r\n  - [x] Potential\r\n  - [x] planarPotential and linearPotential\r\n - [x] Support *any* input to *any* Potential method as Quantity\r\n - [x] Interpolated potential should inherit ``ro`` and ``vo``\r\n - [x] Should we also return actions etc. in ``actionAngle`` using Quantities? Straightforward, but perhaps tedious?\r\n  - [x] Re-write functions so they all use common ``__call__``, ``actionsFreqs``, ``actionFreqsAngles``? This should get units\r\n  - [x] Re-write setup such that they run the general actionAngle setup that sets ``ro=`` and ``vo=``\r\n  - [x] Do the estimateDelta or estimateb functions return proper physical outputs for potentials that have units?\r\n  - [x] Inputs to methods can have units\r\n  - [x] Other setup inputs with units (e.g., ``b`` or ``delta``, ``tintJ``)\r\n  - [x] Inputs to methods that can have units (e.g., ``ts`` for actionAngleIsochroneApprox)\r\n  - [x] Check that the units of given potential are the same as those of the ``actionAngle`` object\r\n  - [x] Check that the units of a given orbit are the same as those of the ``actionAngle`` object?\r\n - [ ] DFs using Quantities\r\n   - [ ] Setup needs to handle ``ro`` and ``vo`` below; make general?\r\n   - [ ] Inputs\r\n   - [ ] Outputs\r\n   - [ ] Sample\r\n   - [ ] Setup of each df\r\n\r\nRequires documentation tasks:\r\n- [ ] Describe configuration file\r\n- [x] Configuration module API\r\n- [ ] Adding new potentials needs to take care of ``ro=`` and ``vo=``\r\n- [ ] Severe re-write of Orbit initialization section\r\n- [ ] Method documentation should make clear that Quantities are allowed\r\n- [ ] Re-write of ``units in galpy`` section\r\n- [ ] Need some documentation on potential functions\r\n- [ ] documentation for new way of initializing potentials?\r\n- [ ] Specify for each potential what the units of its amplitude are\r\n- [x] Methods and functions to turn potential physical output on/off\r\n\r\nOpen issues:\r\n - [ ] Plotting doesn't seem to work with Quantities, even with visualization support from astropy\r\n - [ ] What if the user doesn't provide scales ``ro`` and ``vo``? Error or just use something reasonable? Use default from a configuration file?\r\n - [ ] What to do if the user doesn't provide a scale and asks for ra? If unitless input is given, this should probably at least raise a warning.\r\n - [ ] When time to an Orbit method is just a unitless number, the time according to that unitless number is returned; this might not be the intended use if the integration times were specified using Quantities. Warning or Error?"},"comment":{"url":"https://api.github.com/repos/jobovy/galpy/issues/comments/213597674","html_url":"https://github.com/jobovy/galpy/pull/258#issuecomment-213597674","issue_url":"https://api.github.com/repos/jobovy/galpy/issues/258","id":213597674,"user":{"login":"coveralls","id":2354108,"avatar_url":"https://avatars.githubusercontent.com/u/2354108?v=3","gravatar_id":"","url":"https://api.github.com/users/coveralls","html_url":"https://github.com/coveralls","followers_url":"https://api.github.com/users/coveralls/followers","following_url":"https://api.github.com/users/coveralls/following{/other_user}","gists_url":"https://api.github.com/users/coveralls/gists{/gist_id}","starred_url":"https://api.github.com/users/coveralls/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/coveralls/subscriptions","organizations_url":"https://api.github.com/users/coveralls/orgs","repos_url":"https://api.github.com/users/coveralls/repos","events_url":"https://api.github.com/users/coveralls/events{/privacy}","received_events_url":"https://api.github.com/users/coveralls/received_events","type":"User","site_admin":false},"created_at":"2016-04-22T21:42:51Z","updated_at":"2016-04-22T21:42:51Z","body":"\n[![Coverage Status](https://coveralls.io/builds/5899536/badge)](https://coveralls.io/builds/5899536)\n\nCoverage decreased (-0.3%) to 99.385% when pulling **3e006acb06b2edc503b74ad55f2c5b4ae362da99 on apy-units** into **5b5ff62a7f442c183de9332e9596ac8d14da9179 on master**.\n"}}