DAMD-97: Add missing 2D pipeline outputs #281
Merged
Conversation
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
Moved I/O code to datamodel, so now we need to piggyback on that. Changed so that the Table class is built on top of the pfs.datamodel.PfsTable class, which provides the schema and I/O functions that we delegate to. Table uses the PfsTable schema to set up the properties and a dynamically created RowClass. We use this scheme instead of inheritance because we want to base Table on pandas.DataFrame instead of numpy.ndarray (Table needs to be performant, whereas PfsTable just needs to do I/O). In the process, changed ArcLineSet so that it uses "flux" and "fluxErr" instead of "intensity" and "intensityErr". ReferenceLineSet continues to use "intensity", since those numbers could be anything, whereas ArcLineSet's intensity measurements are actually flux that we measure.
black does it, so let's allow it.
hassanxp
approved these changes
Jul 29, 2022
| @@ -24,14 +35,44 @@ class FocalPlaneFunction(ABC): | |||
| A model of some spectrum as a function of position on the focal plane. | |||
|
|
|||
| This is an abstract base class. Subclasses need to override: | |||
| * ``DamdClass`` class variable: subclass of `pfs.datamodel.PfsTable` that | |||
There was a problem hiding this comment.
For this case, DamdClass is a subclass of PfsFocalPlaneFunction, not PfsTable
| return super().__getattribute__(name) | ||
|
|
||
| def __setattr__(self, name: str, value: Any): | ||
| return super().__setattr__(name, value) |
There was a problem hiding this comment.
Is there a corresponding LSST ticket to fix this permanently in the stack?
I'm sick of getting mypy errors when looking up Struct attributes.
Pushing I/O down to datamodel requires a bit of a redesign. We want the I/O only in the datamodel classes; functional code belongs in drp_stella. But I want to avoid multiple inheritance because it's annoying. So instead of inheriting from the datamodel class, compose it and forward most attribute lookups there. So the datamodel class holds the data, the drp_stella class holds the datamodel class and uses that to do the functional parts. The refactored code has been reformatted with black and is mypy-clean.
PaulPrice
force-pushed
the
tickets/DAMD-97
branch
from
e677710
to
62f4701
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.