Add redshift correction to velocity conversions. #491
Conversation
|
Leaving a quick follow-up: as discussed in #455 this (along with a few others) triggered a discussion of where One way that's specifically relevant to this PR is that we won't have |
|
More concretely: I think the main change required to this PR is to implement |
| (like template fitting) that set this attribute when they are run on | ||
| a spectrum. | ||
| """ | ||
| return self._radial_velocity/cnst.c |
There was a problem hiding this comment.
| return self._radial_velocity/cnst.c | |
| return self._radial_velocity/cnst.c | |
| if val is None: | ||
| self._radial_velocity = None | ||
| else: | ||
| self.radial_velocity = val * cnst.c |
There was a problem hiding this comment.
This assumption is really only true for small redshifts. Should we use the stricter definition?
| (like template fitting) that set this attribute when they are run on | ||
| a spectrum. | ||
| """ | ||
| return self._radial_velocity |
There was a problem hiding this comment.
| return self._radial_velocity | |
| return self._radial_velocity | |
This PR addresses in part issue #258. It might, or might not be the correct interpretation of what this issue was intended to address in the first place. I understood the goal is to apply a known redshift correction to the
velocityattribute ofSpectrum1Dinstances.The implementation uses the simple low-z approximate definition of redshift. It remains to be seen if this is what is actually intended. And, if not, we need more specific requirements on how to interpret redshift values, and what to do with them.