-
-
Notifications
You must be signed in to change notification settings - Fork 39
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
Concept of longslit workflow as an example of specreduce block structure #71
Draft
eteq
wants to merge
1
commit into
astropy:main
Choose a base branch
from
eteq:specreduce-flow-concept
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,122 @@ | ||
""" | ||
This is a conceptual set of classes meant to encode the basics of a longslit | ||
workflow as swappable blocks. Adapted from a whiteboard diagram from | ||
the 2019 Astropy Coordination Meeting (included in the PR adding this file). | ||
|
||
Two things that this file is *not* meant to imply: | ||
|
||
* The specific use or not of classes as opposed to other approaches | ||
for defining interfaces. | ||
* Details of how the various blocks connect together. There are a variety of | ||
"pipeline" packages in the wider community, and we shouldn't be in the | ||
business of duplicating that if we can reuse it. So these blocks just assume | ||
the simplistic idiom of "call the block, then manually pass it's result to the | ||
next one", without anything about how you compose blocks together | ||
|
||
""" | ||
from dataclasses import dataclass | ||
from typing import Union | ||
from pathlib import Path | ||
|
||
|
||
@dataclass | ||
class CCDProc: | ||
""" | ||
Note this class may not need to exist - we could just tell the users "do | ||
whatever you need to do with ccdproc to get something that's been | ||
overscan, bias, dark, and pixel-flat subtracted, but that results in a | ||
""" | ||
ccdproc_subtract_background : bool | ||
#... other ccdproc options ... | ||
|
||
|
||
def __call__(self, rawin: Union([CCDImage,NDData])) -> CCDImage: | ||
raise NotImplementedError() | ||
return ccdproc_corrected_image | ||
|
||
|
||
@dataclass | ||
class CCDProc: | ||
""" | ||
Note this class may not need to exist - we could just tell the users "do | ||
whatever you need to do with ccdproc to get something that's been | ||
overscan, bias, dark, and pixel-flat subtracted, but that results in a | ||
""" | ||
ccdproc_subtract_background : bool | ||
#... other ccdproc options ... | ||
|
||
|
||
def __call__(self, rawin: Union([CCDImage,NDData])) -> CCDImage: | ||
raise NotImplementedError() | ||
return ccdproc_corrected_image | ||
|
||
@dataclass | ||
class AutoIdentify2D: | ||
line_list : dict # maps a name of a line to a *spectral axis unit* astropy quantity | ||
|
||
|
||
def __call__(self, arcimage: CCDImage) -> WavelengthSolution: | ||
raise NotImplementedError() | ||
return solution | ||
|
||
@dataclass | ||
class AutoIdentify1D: | ||
template_spectrum : Union([Path, str]) # should be a path to whatever is used as the template, | ||
#,,, other autoidentify parameters | ||
|
||
def __call__(self, arcimage: CCDImage) -> WavelengthSolution: | ||
raise NotImplementedError() | ||
return solution | ||
|
||
@dataclass | ||
class InteractiveIdentify1D: | ||
""" | ||
This is left generally undefined in detail since it requires some form of | ||
GUI interaction. In principle it may not even best be thought of as a "call | ||
and get result" But the key point is that the user starts with the arc as a | ||
CCDImage and ends with a wavelength solution. | ||
""" | ||
def __call__(self, arcimage: CCDImage) -> WavelengthSolution: | ||
raise NotImplementedError() | ||
return solution | ||
|
||
class WavelengthSolution: | ||
def __init__(self, tbd): | ||
raise NotImplementedError() | ||
|
||
def get_wcs(self): | ||
raise NotImplementedError() | ||
|
||
def apply_wcs(self, spectrum : Union([Spectrum1D, SpectrumCollection])): | ||
spectrum.wcs = self.get_wcs() | ||
|
||
|
||
@dataclass | ||
class BoxcarExtract: | ||
width : float # pixels | ||
trace : Union([astropy.modeling.models.Model1D, float]) # model maps x to y, float is syntactic sugar for a constant model | ||
background_width: Union([float, None]) # None for no background subtraction | ||
background_fit: astropy.modeling.models.Model1D = astropy.modeling.models.Linear1D # the line-by-line background to subtract | ||
|
||
def __call__(self, spec2d: SpectrumCollection) -> Spectrum1D # note spec2d should already have the wavelength solution in it (see the workflow for how this could be acheived) | ||
raise NotImplementedError() | ||
return extracted_spectrum | ||
|
||
|
||
def basic_workflow(): | ||
rawframe = CCDImage.open(fn) | ||
|
||
ccdproc = CCDProc() # whatever needs to happen here | ||
processed_sci = ccdproc(rawframe) | ||
|
||
ccdproc = CCDProc() # whatever needs to happen here | ||
processed_arc = ccdproc(rawdata[idx_arc] | ||
|
||
id2d = AutoIdentify2D(my_line_list_that_i_like) | ||
wlsoln = id2d(processed_arc) | ||
|
||
wlcal_spec = wlsoln.apply(SpectrumCollection([for row in processed_sci])) | ||
|
||
bex = BoxcarExtract(width=10, trace=123*u.pixel, background_width=5) | ||
|
||
spec1d = bex(wlcal_spec) # 1d spectrum! |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to duplicate the above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, that's an obvious duplicate. i also think that this is best left to the
ccdproc
package or whatever a pipeline does to calibrate out detector-level level effects. spectroscopy-specific steps should start with aCCDImage
and go from there.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops yes that's a mistake. But to clarify, I intended exactly what @tepickering said here: this should only wrap ccdproc functionality at best, and might literally just be ccdproc calls