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

Spectrum.smooth does not preserve velocity frame and doppler convention. #417

Closed
mpound opened this issue Oct 25, 2024 · 3 comments
Closed
Assignees
Labels
bug Something isn't working

Comments

@mpound
Copy link
Collaborator

mpound commented Oct 25, 2024

Describe the bug
When you smooth a Spectrum, the resulting Spectrum does not inherit the velocity_frame, doppler_convention, observer, or _observer_location of the original spectrum.

How to Reproduce

  1. velocity_frame, doppler_convention, and observer
from dysh.fits import GBTFITSLoad
from dysh.util import get_project_testdata
f = get_project_testdata()/'TGBT21A_501_11/TGBT21A_501_11_scan_152_ifnum_0_plnum_0.fits'
g = GBTFITSLoad(f)
s = g.getspec(1)
print(f"{s.velocity_frame=}\n{s.doppler_convention=}\n{s.observer=}\n{s._observer_location=}")

s.velocity_frame='itrs'
s.doppler_convention='optical'
s.observer=<ITRS Coordinate (obstime=2021-02-10T07:38:37.500, location=(0., 0., 0.) km): (x, y, z) in m
(882593.9465029, -4924896.36541728, 3943748.74743984)
(v_x, v_y, v_z) in km / s
(0., 0., 0.)>
s._observer_location=None

t = s.smooth("gaussian",16)
print(f"{t.velocity_frame=}\n{t.doppler_convention=}\n{t.observer=}\n{t._observer_location=}")

t.velocity_frame='fk5'
t.doppler_convention='optical'
t.observer=None
t._observer_location=None

  1. observer_location
from dysh.spectra import Spectrum
from dysh.coordinates import Observatory
# use Spectrum `s` from example above.
s2 =  Spectrum(s.flux,observer_location=Observatory["ALMA"],meta=s.meta)
print(f"{s2._observer_location=}")

<EarthLocation (882593.9465029, -4924896.36541728, 3943748.74743984) m>

t2 = s2.smooth('gaussian',16)
print(f"{t2._observer_location=}") 

t2._observer_location=None

Environment

  • Dysh version 0.4.x
  • Python version 3.10
  • OS Ubuntu 22.04
@mpound mpound added the bug Something isn't working label Oct 25, 2024
@astrofle
Copy link
Collaborator

astrofle commented Dec 3, 2024

@mpound I'm a bit conflicted about this. The two constructors for Spectrum can result in different behaviors which are not consistent. Spectrum.make_spectrum() will not add a observer_location attribute as is. Should we add that option or remove observer_location from Spectrum.__init__?

@astrofle
Copy link
Collaborator

astrofle commented Dec 5, 2024

Some notes:

  • Removing Spectrum._observer_location does not break any tests.
  • All Spectrum objects are created using the Spectrum.make_spectrum class method.

Based on this, it seems safe to remove Spectrum._observer_location and keep Spectrum._observer.

@astrofle astrofle self-assigned this Dec 5, 2024
@astrofle astrofle mentioned this issue Dec 5, 2024
@astrofle
Copy link
Collaborator

This was solved by PR #436

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants