adding dataplots.jl #385
adding dataplots.jl #385
Conversation
|
This looks fantastic and I am really excited to get these in There is a potential generalization I mention but maybe we can work on that after this is merged. |
src/visualizations/dataplots.jl
Outdated
| obsdomain = domain(obsdata) | ||
| u = getfield.(getfield(obsdomain,:dims),:U) | ||
| v = getfield.(getfield(obsdomain,:dims),:V) | ||
| uvdist = hypot.(u,v) |
There was a problem hiding this comment.
We have a dedicated function for uvdist
uvd = uvdist.(dt)should work here
src/visualizations/dataplots.jl
Outdated
| end | ||
| elseif field in (:U, :V, :Ti, :Fr) | ||
| obsdomain = domain(obsdata) | ||
| return getfield.(getfield(obsdomain,:dims),field) |
There was a problem hiding this comment.
Again we should do something like
bls = dt.baseline
return getproperty(bls, field)
src/visualizations/dataplots.jl
Outdated
|
|
||
| NamedTuple of the scatter plot keyword arguments | ||
| """ | ||
| function plotfields(obsdata::EHTObservationTable{}, field1::Symbol, field2::Symbol; axis_kwargs=(;), legend_kwargs=(;), scatter_kwargs=(;)) |
There was a problem hiding this comment.
We can replace EHTObservationTable{} with just EHTObservationTable. The {} shouldn't be needed.
src/visualizations/dataplots.jl
Outdated
|
|
||
| νlist = unique(Fr) # multifrequency support | ||
|
|
||
| with_theme(theme_latexfonts()) do |
There was a problem hiding this comment.
I'm a little hesitant to force this specific theme here, just because if someone wants to use a different theme, i.e. different global font sizes, this will not longer be permitted. I think we should probably just leave this off for now, and make an explicit example in the docs if you want LaTeX fonts.
src/visualizations/dataplots.jl
Outdated
| end | ||
| end | ||
|
|
||
| function plotfields(obsdata::EHTObservationTable{}, field1::Symbol, field2::Symbol, site1::Symbol, site2::Symbol; axis_kwargs=(;), legend_kwargs=(;), scatter_kwargs=(;)) |
There was a problem hiding this comment.
Again the {} isn't really needed here.
src/visualizations/dataplots.jl
Outdated
| amps = abs.(vis) | ||
| return amps | ||
| elseif field == :phase # calculate visibility phases | ||
| phases = atan.(imag(vis), real(vis)) |
There was a problem hiding this comment.
| phases = atan.(imag(vis), real(vis)) | |
| phases = angle.(vis) |
will construct the complex argument with the same definition the EHT uses
src/visualizations/dataplots.jl
Outdated
| phases = atan.(imag(vis), real(vis)) | ||
| return phases | ||
| elseif field == :snr | ||
| snr = abs.(vis) ./ obsdata.noise |
There was a problem hiding this comment.
| snr = abs.(vis) ./ obsdata.noise | |
| sigma = Comrade.noise(obsdata) | |
| snr = abs.(vis) .* inv.(sigma) |
just so we keep to the standard format
src/visualizations/dataplots.jl
Outdated
| obsdomain = domain(obsdata) | ||
| u = getfield.(getfield(obsdomain,:dims),:U) |
There was a problem hiding this comment.
getfield should typically be avoided since it signals internals that may not be guaranteed to work for minor revision bumps. For this we should extract the datatable and then grab the elements. So something like
| obsdomain = domain(obsdata) | |
| u = getfield.(getfield(obsdomain,:dims),:U) | |
| dt = datatable(obsdomain) | |
| u = dt.baseline.U | |
| v = dt.baseline.V |
src/visualizations/dataplots.jl
Outdated
| obsdomain = domain(obsdata) | ||
| u = getfield.(getfield(obsdomain,:dims),:U) | ||
| v = getfield.(getfield(obsdomain,:dims),:V) | ||
| uvdist = hypot.(u,v) |
There was a problem hiding this comment.
| uvdist = hypot.(u,v) | |
| uvd = uvdist.(dt) |
We actually have a dedicated for this
src/visualizations/dataplots.jl
Outdated
|
|
||
| :snr - signal to noise ratio | ||
| """ | ||
| function getobsdatafield(obsdata::EHTObservationTable{}, field::Symbol) |
There was a problem hiding this comment.
Looking at this function is isn't quite guaranteed to give the things we want for all EHTObservationTables. The first step is probably to extract the table in a more friendly format, using the datatable function. This tries its best to uniformize everything into a flat table. The columns are usually
| baseline | measurement | noise |
where baseline itself is a table with all the elements, i.e. U, V, Ti, Fr, etc.
|
The plotting code has now been moved to an extension with all your suggestions implemented! (if I blew up your email with these commits i am so sorry) |
| end | ||
| end | ||
|
|
||
| function plotcaltable( |
There was a problem hiding this comment.
Also forgot to mention, I also wrote a function for automatically plotting caltables here!
Comrade plotting functionality for observation data. Currently supports Makie figures and Makie subplots.