{"action":"created","issue":{"url":"https://api.github.com/repos/astropy/astropy/issues/6169","repository_url":"https://api.github.com/repos/astropy/astropy","labels_url":"https://api.github.com/repos/astropy/astropy/issues/6169/labels{/name}","comments_url":"https://api.github.com/repos/astropy/astropy/issues/6169/comments","events_url":"https://api.github.com/repos/astropy/astropy/issues/6169/events","html_url":"https://github.com/astropy/astropy/pull/6169","id":234687262,"number":6169,"title":"Allow representations to contain differentials","user":{"login":"adrn","id":583379,"avatar_url":"https://avatars3.githubusercontent.com/u/583379?v=3","gravatar_id":"","url":"https://api.github.com/users/adrn","html_url":"https://github.com/adrn","followers_url":"https://api.github.com/users/adrn/followers","following_url":"https://api.github.com/users/adrn/following{/other_user}","gists_url":"https://api.github.com/users/adrn/gists{/gist_id}","starred_url":"https://api.github.com/users/adrn/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/adrn/subscriptions","organizations_url":"https://api.github.com/users/adrn/orgs","repos_url":"https://api.github.com/users/adrn/repos","events_url":"https://api.github.com/users/adrn/events{/privacy}","received_events_url":"https://api.github.com/users/adrn/received_events","type":"User","site_admin":false},"labels":[{"id":21499868,"url":"https://api.github.com/repos/astropy/astropy/labels/coordinates","name":"coordinates","color":"c7def8","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/astropy/astropy/milestones/42","html_url":"https://github.com/astropy/astropy/milestone/42","labels_url":"https://api.github.com/repos/astropy/astropy/milestones/42/labels","id":2169283,"number":42,"title":"v2.0.0","description":"","creator":{"login":"astrofrog","id":314716,"avatar_url":"https://avatars1.githubusercontent.com/u/314716?v=3","gravatar_id":"","url":"https://api.github.com/users/astrofrog","html_url":"https://github.com/astrofrog","followers_url":"https://api.github.com/users/astrofrog/followers","following_url":"https://api.github.com/users/astrofrog/following{/other_user}","gists_url":"https://api.github.com/users/astrofrog/gists{/gist_id}","starred_url":"https://api.github.com/users/astrofrog/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/astrofrog/subscriptions","organizations_url":"https://api.github.com/users/astrofrog/orgs","repos_url":"https://api.github.com/users/astrofrog/repos","events_url":"https://api.github.com/users/astrofrog/events{/privacy}","received_events_url":"https://api.github.com/users/astrofrog/received_events","type":"User","site_admin":false},"open_issues":34,"closed_issues":197,"state":"open","created_at":"2016-12-01T16:34:27Z","updated_at":"2017-06-17T12:38:59Z","due_on":"2017-06-17T07:00:00Z","closed_at":null},"comments":21,"created_at":"2017-06-08T23:56:00Z","updated_at":"2017-06-17T14:04:40Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/astropy/astropy/pulls/6169","html_url":"https://github.com/astropy/astropy/pull/6169","diff_url":"https://github.com/astropy/astropy/pull/6169.diff","patch_url":"https://github.com/astropy/astropy/pull/6169.patch"},"body":"This is a stepping stone PR towards adding support for proper motions and velocities in `astropy.coordinates`. It turns out, we need to either (1) allow `Representation`s to carry around `Differential`s, or (2) create a new container class for representation+differentials. Option 2 seems better from a design perspective, but would lead to some API-breaking changes (or else a lot of really annoying code to provide backwards-compatibility). As this only really affects the lower-level API (and not the `Frame` or `SkyCoord`-level API), we decided that, in the interest of time, we would make this \"hack\" for now and just clearly document that the low-level API could change.\r\n\r\nThis PR adds the necessary code to allow `Representation`'s to accept any number of `Differential`'s in their initializers. For simplicity, we are very strict:\r\n* The `Differential` instances must have the same shape as the representation. (I could see loosening this a little and just requiring that they are broadcastable on initialization, but leave that as an open question)\r\n* Shape-changing (e.g., `_apply()`-related operations) affect both the representation and the differentials\r\n\r\nQuestions / TODO's for this PR:\r\n1. [x] How do we modify the `__repr__()` on `BaseRepresentation` when there are differentials associated?\r\n2. [x] Where should I document the subtleties of this? We want to document it somewhere, but also we don't really want people to use it explicitly, or at least have a clear warning on top. Should I just add a section to `docs/coordinates/representations.rst` near the bottom?\r\n3. [x] One slightly annoying side effect of this is that any subclass of `BaseRepresentation` has to remember to pass through the differentials in `to_cartesian()` and `from_cartesian()` (see examples in this PR, e.g., `SphericalRepresentation`. Should we just accept that and document it? EDIT: no, I added new helper methods and changed what subclasses need to implement. This could be API-breaking for anyone who has implemented custom representations, so, what should we do? See [this commit](https://github.com/astropy/astropy/pull/6169/commits/caa177c0589d1acc809de5c406258f816c8082a8) for relevant changes.\r\n4. [x] \"combine\" or \"scale\" operations on representations that contain differentials now raise an error (see the diff for [this commit](https://github.com/astropy/astropy/pull/6169/commits/25a8482072243eb89c84e4f0b4fef144600a5d7d) to see the changes)\r\n5. [x] Write tests for representation arithmetic / methods to make sure the relevant operations raise errors when there are differentials\r\n\r\nCheck out this document if you're interested in seeing a very messy document that lays out some more details: https://docs.google.com/document/d/1dZIG7CG8h-C4lcegtx71eUc76K5LHtgRBsXk4iRa2ds/edit?usp=sharing\r\n"},"comment":{"url":"https://api.github.com/repos/astropy/astropy/issues/comments/309217073","html_url":"https://github.com/astropy/astropy/pull/6169#issuecomment-309217073","issue_url":"https://api.github.com/repos/astropy/astropy/issues/6169","id":309217073,"user":{"login":"adrn","id":583379,"avatar_url":"https://avatars3.githubusercontent.com/u/583379?v=3","gravatar_id":"","url":"https://api.github.com/users/adrn","html_url":"https://github.com/adrn","followers_url":"https://api.github.com/users/adrn/followers","following_url":"https://api.github.com/users/adrn/following{/other_user}","gists_url":"https://api.github.com/users/adrn/gists{/gist_id}","starred_url":"https://api.github.com/users/adrn/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/adrn/subscriptions","organizations_url":"https://api.github.com/users/adrn/orgs","repos_url":"https://api.github.com/users/adrn/repos","events_url":"https://api.github.com/users/adrn/events{/privacy}","received_events_url":"https://api.github.com/users/adrn/received_events","type":"User","site_admin":false},"created_at":"2017-06-17T14:04:40Z","updated_at":"2017-06-17T14:04:40Z","body":"@mhvk On the idea of `_compatible_differentials`: unfortunately this doesn't work without some major reorganization of the module because the differential classes are defined below the representations. I didn't think of any other easy alternatives, so I could split out the differential classes into a separate module and try to resolve any circular imports...thoughts?"}}
{"actor":{"display_login":"adrn"}}