Adding an exptime calculator, Telescope class, and sky model

[tcjansen]

All of the additions I've made in this PR are motivated by adding an exposure time calculator to astroplan. To do so, the user supplies their own spectrum (or spectral model) of their target (in the following example as waveset_of_target and flux_of_target), the desired signal to noise ratio (SNR) the user wants to reach, an astroplan observer object, and a new astroplan telescope object to a function called exptime_from_ccd_snr:

>>> from astroplan import exptime_from_ccd_snr
>>> snr = 100
>>> exptime_from_ccd_snr(snr, waveset_of_target, flux_of_target, observer, telescope)

To compute the exposure time needed to obtain the given SNR, astroplan needs to know some telescope specifications such as the gain, aperture area, and bandpass. The new astroplan.telescope.Telescope object handles those parameters. astroplan then convolves the bandpass, spectrum, and any other optional effects (such as ccd response) with the help of synphot, which astroplan also uses to get the count rate of the mock observation.

I have also added the option for the user to input a custom sky model to the Observer object (i.e. a model of the atmospheric transmission at the location of the observer). With the new skycalc module, the user can alternatively obtain a sky model generated from an arsenal of optional sky parameters via the SKYCALC sky calculator.

See here for a tutorial!

Much of the content of this PR has been reviewed in this PR to synphot, where we had originally thought these functions might go before deciding that astroplan would be more a more appropriate place!

I haven't integrated the tests yet since I wanted to get this PR in ASAP, but they are coming soon (after which some minor changes may be made if I discover any buggy behavior)!

Update: this PR closes #29

[pllim]

Test failures are symptoms of a broken CI on master itself. I am trying to see if I can fix it up at #421 but I can't promise anything.

UPDATE: Yup, I fixed it, but it needs to be merged first before you can reap the benefit here via a rebase.

[pllim]

Thank you, @tcjansen ! Indeed, the workflow looks much better as part of astroplan, at least to me.

As I have reviewed the rest before, just some minor comments remain and a thought about caching the download.

Comments for the notebook

• Remove commented # warnings.filterwarnings("ignore", module='astropy.io.votable.tree').
• Fix an extraneous </tt> showing in rendered notebook at the beginning of Section 3.

[pllim]

I wonder if astroplan has some sort of caching mechanism so that calling this function repeatedly will not result in a network call if a previously downloaded file exists.

[tcjansen]

@bmorris3? I don't know anything about how caching works in python (or otherwise.. 😬)

[bmorris3]

astroplan doesn't do any caching to prevent network calls yet, but we can imagine making a cache specifically for this kind of query.

[tcjansen]

Thanks for looking it over @pllim! Sorry it was unusually sloppy, I forgot to run pycodestyle before pushing 😱 (I used to have my sublimelinter running but it has mysteriously stopped showing me errors...)

[pllim]

@tcjansen , tests are fixed for master, please rebase. Thanks!

[bmorris3]

To follow up: did the SkyCalc CLI folks ever get back to you via email about adding it to astroquery? I still think that's the best way to go for the future of this particular method.

[tcjansen]

They did! They said I should feel free to go ahead and make a PR to astroquery, and that their "software developers would surely consider it" - not sure if that means they'd chime in on my PR or if they'd consider making their own PR at some point, but either way it sounds like they gave me the green light.

[bmorris3]

Is this comment still necessary?

[tcjansen]

Ah, yeah, this is actually kind of an important distinction. The issue arises when the source spectrum and bandpass don't fully overlap - you can tell synphot to force the convolution anyway, either by making the incomplete spectral component taper to zero for its missing wavelengths, or by guessing/extrapolating what the remainder of the spectrum will be. Extrapolating might be okay if only a tiny bit of the spectrum is missing, but otherwise sounds a bit risky to me, which is why I chose to taper.

All that said, I think the best approach would be to add an optional parameter like force_overlap to exptime_from_ccd_snr() with a default of taper, and perhaps raise a warning if this forcing is implemented (which synphot might already do, actually). That way the user could both be aware that they don't overlap and could decide for themselves how they want to handle it.

[bmorris3]

Hey @tcjansen – any chance you'll be able to address the comments above sometime? If not, no problem just let us know so someone else gets to it. Thanks again!

[tcjansen]

Ohh my gosh thanks for the reminder, @bmorris3! For some reason I had it in my head that I was waiting for comments... Will get to it soon!

[bmorris3]

Added some more comments – please also rebase when you get a chance.

[bmorris3]

In order to get your new tests to pass which rely on synphot, you'll need to do a few more small changes:

• add a line like the others here for synphot
• add synphot to the tests_require line here
• add synphot to the testing matrix dependencies here

[tcjansen]

• add synphot to the testing matrix dependencies here

So I tried adding synphot to this line but I'm getting a conflict error... what's the correct way of going about this?

[bmorris3]

You'll need to rebase and then force-push your changes. I recommend you make a backup branch of your work by typing git branch backup before you rebase, just in case.

[tcjansen]

Ah okay, I misunderstood "rebase" to mean "squash". Now that the conflicts are fixed, travis is still angry... I tried a few different approaches to adding synphot to '.travis.yml' but no luck.. but I'm admittedly fumbling around in the dark since I'm not familiar with travis 😅

[bmorris3]

Can you please move this line to the inside of the test function that calls the Gaia module? It's part of the reason why the tests are failing.

[bmorris3]

Sorry to leave you hanging so long @tcjansen!

• One test is failing because astroquery is not in pip requirements for tests. You might be able to fix this by adding astroquery to this line

• Another test is failing due to formatting of the file. To reproduce this test locally, install flake8 with pip install flake8 type the following in the astroplan repo flake8 astroplan --count --max-line-length=100. It will return to you the formatting tweaks that you need to make. Adjust the code and rerun flake8 until it returns 0.

• this one is a bit mysterious to me, but maybe we'll tackle it after we get the first two solved.