Skip to content

Add documentation for velocity support in coordinates#6260

Merged
eteq merged 33 commits intoastropy:masterfrom
adrn:coordinates/frame-velocity-docs
Jul 5, 2017
Merged

Add documentation for velocity support in coordinates#6260
eteq merged 33 commits intoastropy:masterfrom
adrn:coordinates/frame-velocity-docs

Conversation

@adrn
Copy link
Member

@adrn adrn commented Jun 22, 2017

This is a companion to #6219 to add / update the astropy.coordinates documentation to explain how velocities are incorporated. This is mainly just so the files changed in #6219 doesn't get even more out of hand...

@eteq
Copy link
Member

eteq commented Jun 22, 2017

@eteq
Copy link
Member

eteq commented Jun 22, 2017

@mhvk
Copy link
Contributor

mhvk commented Jun 24, 2017

Just had a look, and like the initial update. I think we'd need some examples, though, which ideally show some obviously correct transformations (like a PM in pure galactic longitude near the galactic centre, so one sees where it is heading in RA, Dec).

@adrn
Copy link
Member Author

adrn commented Jun 27, 2017

Also, as discussed in #6219, clean up mentions of "must be keyword" in Frame class docstrings.

@eteq
Copy link
Member

eteq commented Jun 27, 2017

  • Synergize with placeholder "what's new" section on the velocities

eteq added a commit that referenced this pull request Jun 27, 2017
whatsnew-2.0-coordinates-velocities is a placeholder, to be updated further in #6260
eteq added a commit that referenced this pull request Jun 27, 2017
whatsnew-2.0-coordinates-velocities is a placeholder, to be updated further in #6260
@bsipocz
Copy link
Member

bsipocz commented Jun 27, 2017

As this is supposed to be docs only I'll go ahead and cut the RC before it. But I'll wait a few more hours in the hope that either @adrn or @eteq wakes up and can rebase this.

@adrn
Copy link
Member Author

adrn commented Jun 27, 2017

Right, don't merge yet - we still have some more docs to add!

@bsipocz
Copy link
Member

bsipocz commented Jun 27, 2017

OK, this is useful info as well, so I'll go ahead with the RC and will merge this when ready for the actual release.

@adrn adrn force-pushed the coordinates/frame-velocity-docs branch from 8317b2f to 9bd8d30 Compare June 28, 2017 20:28
@adrn adrn changed the title Add documentation for velocity support in coordinates [WIP] Add documentation for velocity support in coordinates Jun 28, 2017
@adrn adrn mentioned this pull request Jun 29, 2017
@adrn adrn force-pushed the coordinates/frame-velocity-docs branch from 2cc1d38 to ab6951c Compare July 1, 2017 19:29
@adrn
Copy link
Member Author

adrn commented Jul 2, 2017

  • the galactocentric.rst page still needs updating to include information about velocities.
  • we should add an example showing how to set the Galactocentric v_sun with the proper motion of Sgr A* rather than a 3D cartesian velocity

@eteq
Copy link
Member

eteq commented Jul 4, 2017

@adrn - OK, populated "my" sections from #6260 (comment) and those right after, so I think it's just the two you noted above. The "what's new" could maybe use some beefing up, but I'm unsure what to put beyond what's there... open to ideas, but I'm also willing to accept that as it stands here.

@eteq
Copy link
Member

eteq commented Jul 4, 2017

Oh, but one other thing to add either here or in #6317 (whichever gets merged second):

@adrn
Copy link
Member Author

adrn commented Jul 5, 2017

Note: this now contains commits from #6329

@adrn adrn force-pushed the coordinates/frame-velocity-docs branch from 349a8fe to 5a66dc8 Compare July 5, 2017 03:46
@adrn
Copy link
Member Author

adrn commented Jul 5, 2017

I added two examples to this PR, but it really could use one more: an example of how to subclass BaseCoordinateFrame to implement a position and velocity offset (e.g., shift to the Milky Way-M31 barycenter). Not sure I'll have time to implement this before tomorrow, though...

@adrn adrn force-pushed the coordinates/frame-velocity-docs branch from 5a66dc8 to 51aa3bf Compare July 5, 2017 12:13
@astropy-bot
Copy link

astropy-bot bot commented Jul 5, 2017

Hi there @adrn 👋 - thanks for the pull request! I'm just a friendly 🤖 that checks for issues related to the changelog and making sure that this pull request is milestoned and labelled correctly. This is mainly intended for the maintainers, so if you are not a maintainer you can ignore this, and a maintainer will let you know if any action is required on your part 😃.

Everything looks good from my point of view! 👍

@adrn adrn changed the title [WIP] Add documentation for velocity support in coordinates Add documentation for velocity support in coordinates Jul 5, 2017
@eteq
Copy link
Member

eteq commented Jul 5, 2017

Once the tests pass, we should go ahead and merge this so that we have this in (if nothing else, for the what's new), but we can definitely still try to squeeze in another example in a separate PR if there's time before the release is finalized, @adrn

@eteq eteq added zzz 💤 merge-when-ci-passes Do not use: We have auto-merge option now. and removed Work in progress labels Jul 5, 2017
@eteq eteq merged commit a80e1c1 into astropy:master Jul 5, 2017
bsipocz pushed a commit that referenced this pull request Jul 5, 2017
Add documentation for velocity support in coordinates
@MSeifert04 MSeifert04 removed the zzz 💤 merge-when-ci-passes Do not use: We have auto-merge option now. label Oct 17, 2017
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.

5 participants