CALSPEC standards

[cgarling]

@lucasvalenzuela Fixes #27 by supporting arbitrary spectral standards from CALSPEC. Files are automatically downloaded and cached into a scratchspace using Scratch.jl. I don't know if this will work for every single file in the extended CALSPEC catalog but I tried it with all the alpha_lyr files and they all look good. I can write more extended tests if we want to test against a wider range of standard files.

[codecov]

Codecov Report

❌ Patch coverage is 85.18519% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.32%. Comparing base (056df57) to head (82b5987).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
src/core.jl	0.00%	2 Missing ⚠️
src/vega.jl	91.66%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #31      +/-   ##
==========================================
- Coverage   81.76%   80.32%   -1.44%     
==========================================
  Files           6        6              
  Lines         351      366      +15     
==========================================
+ Hits          287      294       +7     
- Misses         64       72       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[cgarling]

Any thoughts on this @lucasvalenzuela ?

[lucasvalenzuela]

Thanks for the ping! I have left some comments on the code.

[lucasvalenzuela]

Since the joinpath call doesn't do anything, I would leave it away

[lucasvalenzuela]

Currently this leads to the unwanted behavior that Base.show for these Vega objects prints the following:
Vega magnitude system with reference spectrum /.../.julia/scratchspaces/.../vega_standards/alpha_lyr_stis_011.fits.

I would either add a second Vega(name, filename) method or automatically extract the name from the filename, though I think the latter approach would be less elegant.

[lucasvalenzuela]

The following should resolve this:

function Vega(name::String = "alpha_lyr_stis_011", fname=splitext(name)[1] * ".fits")
    name = splitext(name)[1]
    if isfile(fname)
        return FITSIO.FITS(fname, "r") do f
            wavelength = read(f[2], "WAVELENGTH") .* u"angstrom"
            flux = read(f[2], "FLUX") .* u"erg/s/cm^2/angstrom"
            Vega(wavelength, flux, name)
        end
    end

    # Load from CALSPEC database or local cache
    fname = joinpath(vega_cache, fname)
    if !isfile(fname)
        Downloads.download("https://ssb.stsci.edu/cdbs/calspec/$name.fits", fname)
    end
    return Vega(name, fname)
end
Vega(name::AbstractString, args...) = Vega(string(name), args...)

[cgarling]

Simply replacing Vega(wavelength, flux, name) with Vega(wavelength, flux, basename(name)) should accomplish this yes? The name in the struct is not used for anything but I think it's nice to track it for user info (e.g., so it's explicit where it came from).

[lucasvalenzuela]

Oh yes, I think you're right. I keep forgetting about all the Filesystem functions in Julia...

[lucasvalenzuela]

Any reason not to use HTTP.jl as it is already a dependency?

[lucasvalenzuela]

I like the scratchspace part of things, that seems very useful. Do you think that would also make sense for the SVO filters?
Apart from that, I was successfully able to load multiple other spectra from Calspec. I don't think it needs extensive tests for that many, as the format seems to be pretty consistent.

I was also wondering if it would be handy to also have a "what's available" function for the spectra, similar to the query_filters function for the SVO Filters? I had a go at it with HTTP.jl and XML.jl to parse the Calspec website directly:

function get_calspec_names()
    response = HTTP.get("https://ssb.stsci.edu/cdbs/calspec/")
    xml = read(IOBuffer(response.body), LazyNode)

    calspec_names = String[]

    for node in xml
        if tag(node) == "a"
            filename = value(children(node)[1])
            calspec_name, ext = splitext(filename)
            ext == ".fits" || continue
            push!(calspec_names, calspec_name)
        end
    end

    return calspec_names
end

What do you think?

[cgarling]

Thanks for the review and contribution! I added your get_calspec_names plus a version that takes an input substring and filters the list to only include names that include substring. Seems like a very common operation (to, e.g., get the list of all "alpha_lyr" spectra).

I am presently against a simple cache for the SVO filters as they are not necessarily static. The filters can be updated over time -- in fact, the detector type for the 2MASS filters was just changed from energy to photon to properly reflect the convention of the underlying filter curves. I like the cache for the CALSPEC stuff because they are generally static as a function of file name -- "alpha_lyr_stis_011.fits" should always contain the same data, and if they want to issue a new standard spectrum, they increment the "011" part.

I can see an avenue for caching the filters that might look something like this: on first request, the SVO filter is downloaded and cached according to its identifier. It is used from the cache for future requests. We provide a method (something like update_filters) that looks into the cache and sees what filters you have there and redownloads all of them from SVO, refreshing the cache and making sure you have the most up-to-date filters.

I don't presently want to work on that, but what do you think?

[lucasvalenzuela]

This looks good, just added two minor comments. I also noticed that Base.show for the Vega objects prints a full stop at the end of the line, which I don't think is generally used for printing types like that.

Besides those three points I think this is good to go!

[lucasvalenzuela]

And that is a good point on the static nature of the spectra vs the filters. I like your idea of letting the user cache the filters and update them every so often. I think I will tackle that when I find time to work on my filter project again (which will most likely still be a few months away, though).

[cgarling]

Thanks! One note on the show method, for me it looks like this

which has the same free line after the output as showing other types like simple numbers

so I'm not sure exactly what you mean.

I think this is good to merge, do you agree?

Additionally, I think this is now feature complete enough to register. What do you think? We can stay at 0.1.0 if you plan on adding the filter caching and maybe we bump to 1.0 after that?

[cgarling]

Actually I think I'll try to work on the filter caching this afternoon so I'm going to merge this. Feel free to open issues if there's anything else from this PR that needs to be fixed.