Repository navigation
Removing location kwarg from moon illumination/phase computations - #213
Merged
Merged
Conversation
…eaning up constraints.py
Contributor
|
HI @bmorris3 I've left a comment on astropy/astropy#5216, but there's a PR for astropy which will fix that issue. That PR has a milestone of v1.2.2. I guess you might want to merge this anyway if you want the next astroplan release to support astropy 1.2 and 1.2.1... |
Contributor
Author
|
I'm going to wait until at least tomorrow to merge. This should also help speed up your constraint computations when the moon illumination is evaluated. |
Closed
This was referenced Aug 5, 2016
|
Hi, it looks like when you removed the location requirement you didn't update the docstrings, so the generated docs (see here) still say you need to include a location. Looks like a straightforward change (remove two lines). Also the same for moon_phase_angle. |
Merged
Member
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR removes the location keyword argument from the
moon_illumination,moon_phase_anglefunctions in themoonmodule.The motivation here is that astropy's
get_moondoes not yet support computations for multiple times at a non-geocentric observer, so we were calculating each moon position with a slowforloop (astropy/astropy#5216). While the location of the observer on the surface of the Earth matters a lot if you want precise moon positions on the celestial sphere, it doesn't affect the result much for computing moon phase/illumination compared to a geocentric observer. So we can get vast speed-ups if we just drop the location specification here, where it doesn't matter much.I've simply removed the
locationkeyword argument, which now computes all phases/illuminations from GCRS=(0, 0, 0).cc @StuartLittlefair @eteq @kvyh