DM-45010: Change RuntimeError to AlgorithmError #379
Conversation
| msg = "There are no valid stars to compute normalized calibration fluxes." | ||
| msg += (f" Of {initNSources} initially selected sources, {nCalibFluxFlag} have good raw calibration " | ||
| f"fluxes and {nRefFluxFlag} have good reference fluxes.") |
There was a problem hiding this comment.
My only concern is that this doesn't seem like it contains quite the information one needs to reconstruct the failure conditions. n_sel==0 is the failure condition, but I don't quite know how the two "good" numbers you give in the error relate to that. Maybe that's ok, I'm just not sure.
There was a problem hiding this comment.
The idea is to tell you which of the selection cuts is causing there to be no sources. My logic is that this is the first thing you would want to know if you encountered this error.
| n_sel = sel.sum() | ||
|
|
||
| if n_sel == 0: |
There was a problem hiding this comment.
This can be a walrus:
| n_sel = sel.sum() | |
| if n_sel == 0: | |
| if (n_sel := sel.sum()) == 0: |
There was a problem hiding this comment.
Nice, I need to build my walrus confidence.
cmsaunders
force-pushed
the
tickets/DM-45010
branch
from
29f4052 to
1f213ed
Compare
|
|
||
| Parameters | ||
| ---------- | ||
| initNSources : `int` |
There was a problem hiding this comment.
Does init here mean initial? Maybe spell that out explicitly, since these names go into the metadata that is written to disk. initialNSources or nInitialSources (or n_initial_sources, since it seems like we're more fully trying to move to snake_case these days?).
There was a problem hiding this comment.
I went with n_initial_sources and also changed the other variables to snake_case because I realized this file is pretty much all snake case already.
| """ | ||
| def __init__(self, *, initNSources, nCalibFluxFlag, nRefFluxFlag): | ||
| msg = "There are no valid stars to compute normalized calibration fluxes." | ||
| msg += (f" Of {initNSources} initially selected sources, {nCalibFluxFlag} have good raw calibration " |
There was a problem hiding this comment.
"raw"? Do you mean "original"? I'm not sure what "raw" means in this context.
There was a problem hiding this comment.
This is the language that is used elsewhere in NormalizedCalibrationFlux. The config that does this selection is called raw_calibflux_name. Do you think it would be more clear to write "good raw_calibflux_name fluxes" or something like that?
|
|
||
| @property | ||
| def metadata(self): | ||
| metadata = {"nInitSources": self.initNSources, |
There was a problem hiding this comment.
I think it's worth being consistent about the variable name and the metadata.
| NormalizedCalibrationFluxError if there are not enough sources to | ||
| calculate normalization. |
There was a problem hiding this comment.
| NormalizedCalibrationFluxError if there are not enough sources to | |
| calculate normalization. | |
| NormalizedCalibrationFluxError | |
| Raised If there are not enough sources to calculate normalization. |
cmsaunders
force-pushed
the
tickets/DM-45010
branch
from
1f213ed to
28eb319
Compare
cmsaunders
force-pushed
the
tickets/DM-45010
branch
from
28eb319 to
35847cf
Compare
No description provided.