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
DM-38398: Fix crashes exposed by rc2_subset. #100
Conversation
@@ -166,7 +166,7 @@ def setDefaults(self): | |||
source_selector = self.sourceSelector["science"] | |||
source_selector.setDefaults() | |||
|
|||
source_selector.doFlags = True | |||
source_selector.doFlags = False |
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.
Did you really mean to turn off (even the default science selector) flag filtering by default?
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.
Yes, this is intentional. When running from the isolated stars the flags have already all been applied in the original isolated star association task, except for the local background flag. What I have now may be redundant I realize, but it does work.
@@ -388,8 +388,11 @@ def _make_all_star_obs_from_isolated_stars( | |||
self.config.apertureOuterInstFluxField + "Err", | |||
] | |||
|
|||
local_background_flag_name = None |
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.
I'm not seeing why this initialization is necessary (as the other use below is also behind a if self.config.doSubtractLocalBackground:
clause)...
) | ||
loop_bands = self.config.requiredBands | ||
else: | ||
loop_bands = np.unique(sources['band']) |
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.
Double vs. single quotes again.
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.
Argh!
fff9ce0
to
8053578
Compare
This PR fixes a flag loading issue, and an index error that can come with very thin data like rc2_subset when all of the observations of a given star are flagged as bad due to having no flux after correcting for the local background.