-
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
HLA-1138: Exclude single filter images in the generation of the total detection image #1797
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1797 +/- ##
==========================================
- Coverage 33.30% 31.45% -1.85%
==========================================
Files 126 160 +34
Lines 31187 35166 +3979
Branches 5770 0 -5770
==========================================
+ Hits 10386 11061 +675
- Misses 19640 24105 +4465
+ Partials 1161 0 -1161 ☔ View full report in Codecov by Sentry. |
drizzlepac/haputils/product.py
Outdated
set_of_filters = set([element.filters for element in self.edp_list]) | ||
count_dict = {key: [] for key in set_of_filters} | ||
|
||
# Loop over the exposure objects to collect the exposures for each filter | ||
for expo in self.edp_list: | ||
for key in count_dict: | ||
if key == expo.filters: | ||
count_dict[key].append(expo.full_filename) | ||
break |
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.
set_of_filters = set([element.filters for element in self.edp_list]) | |
count_dict = {key: [] for key in set_of_filters} | |
# Loop over the exposure objects to collect the exposures for each filter | |
for expo in self.edp_list: | |
for key in count_dict: | |
if key == expo.filters: | |
count_dict[key].append(expo.full_filename) | |
break | |
count_dict = {} | |
# Loop over the exposure objects to collect the exposures for each filter | |
for expo in self.edp_list: | |
count_dict.set_default(expo.filters,[]).append(expo.full_filename)) |
I think the current code is much more complicated than it needs to be. I removed the set_of_filters
variable because I think it is not used elsewhere. If I'm wrong about that, it can be computed from count_dict
after the loop.
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.
Indeed. I am not very Pythonic in my thinking. Also, I forgot that by definition, a dictionary is comprised of a set of unique keys!
image to minimize cosmic ray contamination, unless there are only single filter images in the visit.
added a log message as suggested during the code review.
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.
Looks good to me!
Resolves [HLA-1138(https://jira.stsci.edu/browse/HLA-1138)
Closes #
This PR addresses ...
Exclude single filter images in the generation of the total detection
image to minimize cosmic ray contamination, unless there are only single filter images in the visit.
Checklist for maintainers
CHANGELOG.rst
within the relevant release sectionHow to run regression tests on a PR