Skip to content
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

provide an optional second spectral conversion to the cursor info in the spectral profiler #1837

Closed
kswang1029 opened this issue Apr 26, 2022 · 6 comments · Fixed by #1883
Closed
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@kswang1029
Copy link
Collaborator

kswang1029 commented Apr 26, 2022

given the time pressure of the v3 release, an alternative solution for the feature #1703 (see implementation challenges in #1835) is to just add an additional spectral value based on the user selection to the cursor info in the spectral profiler.

Screen Shot 2022-04-26 at 15 55 32

In the above example, at the moment we show (frequency, intensity). If it is enabled from the settings dialog (default off) and velocity is selected, then we show (frequency, velocity, intensity) instead.

The reference image for freq-velo conversion should be the “active” or user-selected image. This is relevant when multi-profile plot mode is enabled.

quick mockup
Screen Shot 2022-04-26 at 16 44 23

Attached is a doc I compiled from AST user manual regarding spectral and spatial conversion functions. Hope it helps.
carta_ast_specConvert_wcConvert.pdf

@kswang1029 kswang1029 added the enhancement New feature or request label Apr 26, 2022
@kswang1029
Copy link
Collaborator Author

kswang1029 commented Apr 26, 2022

@jrhosk @YuHsuan-Hwang This is one of the possible (and easiest maybe) solutions for #1703 that I can think of. As for the actual implementation of adding a 2nd spectral axis as PR #1835 is doing, there might be another way to achieve similar result by treating x2 and y2 axis as independent axes (based on my understanding from chartjs doc) but this requires R&D time to verify the idea. For the upcoming v3 release, I suggest we try to achieve the cursor info approach and worry about #1703 later on. Please let me know if this is sensible to you. :)

@jrhosk
Copy link
Contributor

jrhosk commented Apr 27, 2022

So the consensus is that I should work on #1703 and leave #1835 until some R&D can be done?

@jrhosk
Copy link
Contributor

jrhosk commented Apr 27, 2022

given the time pressure of the v3 release, an alternative solution for the feature #1703 (see implementation challenges in #1835) is to just add an additional spectral value based on the user selection to the cursor info in the spectral profiler.

Screen Shot 2022-04-26 at 15 55 32

In the above example, at the moment we show (frequency, intensity). If it is enabled from the settings dialog (default off) and velocity is selected, then we show (frequency, velocity, intensity) instead.

The reference image for freq-velo conversion should be the “active” or user-selected image. This is relevant when multi-profile plot mode is enabled.

quick mockup Screen Shot 2022-04-26 at 16 44 23

Attached is a doc I compiled from AST user manual regarding spectral and spatial conversion functions. Hope it helps. carta_ast_specConvert_wcConvert.pdf

The document is great, thank you! Are there other such documents? More documentation would be amazing.

@kswang1029
Copy link
Collaborator Author

So the consensus is that I should work on #1703 and leave #1835 until some R&D can be done?

@jrhosk we should evaluate these two features with @YuHsuan-Hwang and see which one is realistic for v3 final release.

@YuHsuan-Hwang
Copy link
Collaborator

@jrhosk we should evaluate these two features with @YuHsuan-Hwang and see which one is realistic for v3 final release.

I would prefer we work on this issue for v3

@kswang1029
Copy link
Collaborator Author

@jrhosk we should evaluate these two features with @YuHsuan-Hwang and see which one is realistic for v3 final release.

I would prefer we work on this issue for v3

How about this feature goes first and if time allows, revisit #1703 and continue #1835?. The two features have common parts (spectral conversion, specifically) so part of codes in #1835 may be re-used I guess.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants