-
Notifications
You must be signed in to change notification settings - Fork 3
Unify XSpectrum types
#24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #24 +/- ##
==========================================
+ Coverage 74.28% 81.87% +7.58%
==========================================
Files 8 8
Lines 140 171 +31
==========================================
+ Hits 104 140 +36
+ Misses 36 31 -5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
We did this in PhotometricFilters.jl, see here. Is it useful for getindex to return a Base.getindex(f::Spectrum, i::Int) = (wave(f)[i], flux(f)[i]) |
|
Thanks! Yea, I liked how this keeps things consistent with Actually... This may go nowhere, but I think I want to start investigating #25 (comment) now |
|
Ok, I think this might be going somewhere. Updated the parent comment with a summary |
|
If you want to cover specutils' case 2 (multi-dimensional flux, but same spectral axis along each) you may need to add a type parameter for the dimensionality of the wavelength array as well. I think case 2 is for IFU data (??) so maybe mutable struct Spectrum{W<:Number, F<:Number, N, M} <: AbstractSpectrum{W, F}
wave::AbstractArray{W, N}
flux::AbstractArray{F, M}
meta::Dict{Symbol,Any}
end
const SingleSpectrum = Spectrum{W, F, 1, 1} where {W, F}
const IFUSpectrum = Spectrum{W, F, 1, 2} where {W, F}
const EchelleSpectrum = Spectrum{W, F, 2, 2} where {W, F}We can also worry about this later if you want |
|
oh I like this a lot. Added some methods to start playing with this. There's something really satisfying about being able to do, say: julia> spec_IFU = spectrum([20, 40, 120, 160, 200], rand(10, 5))
IFUSpectrum(Int64, Float64)
# orders: 10
wave: (20, 200)
meta: Dict{Symbol, Any}()
julia> spec_IFU[3, begin:4]
SingleSpectrum(Int64, Float64)
wave: (20, 200)
flux: (0.12659667784419948, 0.3942549580490494)
meta: Dict{Symbol, Any}(:Order => 3)to get the first 4 flux measurements from the 3rd order of an |
|
Actually I think an IFU with a uniform wavelength (dispersion) sampling would be a 3-D cube (2 spatial axes across the IFU field, one wavelength axis) so I think it's a good idea, not sure what the best API is. I would probably put the wavelength axis first in the flux matrix because the most common operation is probably selecting a 1-D spectrum from the cube based on pixel. julia> wave, flux = [20, 40, 120, 160, 200], rand(5, 10, 10);
julia> spec_ifu = spectrum(wave, flux);
julia> spec_ifu[:, 1, 1] # Selects full 1-D spectrum from spatial pixel with index (1, 1)My intuition is that a scalar wavelength index would return a basic array (i.e, an image of the flux at a fixed wavelength). julia> spec_IFU[3, begin:4, begin:4] isa Matrix{Float64}
true
julia> spec_IFU[3, begin:4, begin:4] == flux[3, begin:4, begin:4]
trueI think returning a spectrum type only makes sense if the index is a range for the wavelength. I'm really not sure though, I don't really work on this either |
|
Oh cool! Thanks for the reference, I've never worked with IFUs before, so this was a really interesting read. I think "spaxel" is my new favorite word 😂. I see that I was mixing concepts between echelles and ifus when I was talking about orders, sorry about that. The split to 3D really helped clarify things for me. How does this new design look? I also cleaned up the show methods a bit (still need to update tests once this settles though) julia> using Spectra
julia> wave, flux = [20, 40, 120, 160, 200], rand(5, 10, 6);
julia> spec_ifu = spectrum(wave, flux)
IFUSpectrum(Int64, Float64)
wave ((5,)): 20 .. 200
flux ((5, 10, 6)): 0.6211135421955702 .. 0.4976591674645773
meta: Dict{Symbol, Any}()
julia> spec_ifu[:, 1, 1]
SingleSpectrum(Int64, Float64)
wave ((5,)): 20 .. 200
flux ((5,)): 0.6211135421955702 .. 0.9046136130205523
meta: Dict{Symbol, Any}()
julia> spec_ifu[:, begin:4, begin:3]
IFUSpectrum(Int64, Float64)
wave ((5,)): 20 .. 200
flux ((5, 4, 3)): 0.6211135421955702 .. 0.8211549470853685
meta: Dict{Symbol, Any}()
julia> spec_ifu[3, begin:4, begin:3]
4×3 Matrix{Float64}:
0.184341 0.942616 0.546656
0.206153 0.754342 0.120533
0.186781 0.0043856 0.0914326
0.601994 0.352066 0.82047
julia> spec_ifu[3, begin:4, begin:3] == flux[3, begin:4, begin:3]
true |
Doc preview: https://juliaastro.org/Spectra.jl/previews/PR24/
Closes: #25
Currently covering representation 1 and 3 here https://specutils.readthedocs.io/en/latest/types_of_spectra.html
What's changed
Spectrum,EchelleSpectrum, and a newIFUSpectrumfor spectral cube support are now justSpectrums, with their dimensionality specified in the type domain, i.e.,More of the array interface is implemented now so things like this will work:
Merged
common.jlintoSpectra.jl(for my own sanity)Renamed the
Analysispage of the docs toUtilitiesand movedblackbodyover to itFilled in some more API documentation
To-do
Spectrumsmeta(maybe switch to NamedTuple)