Skip to content
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

Skip bad header keywords when making filter combos #187

Merged
merged 2 commits into from
Dec 29, 2023

Conversation

TheSkyentist
Copy link
Contributor

This is a fix for #117 which was briefly mentioned in #182.
The bug only arises when multiple bands are combined to create detection images.

@lboogaard lboogaard mentioned this pull request Dec 13, 2023
@TheSkyentist
Copy link
Contributor Author

These keywords are included in the headers if the drizzled images are made with pipeline.auto_script.make_combined_mosaics and not aws.visit_processor.cutout_mosaic. So this doesn't affect mosaics built with visit_processor. The solution is either to skip the offending keywords as suggested here, or to rewrite the portions of make_combined_mosaics that include these header keywords to begin with.

@gbrammer
Copy link
Owner

I think it's fine to do the skip here, I was just requesting a slight rewording of the if test.

@TheSkyentist
Copy link
Contributor Author

Yes, in this commit I'm only skipping the offending keywords which turn out to be either an empty string or HISTORY, rather than skipping the entire header assignment as was done in the previous PR. Is there still something to be improved?

@gbrammer
Copy link
Owner

Please see the tiny review request above for a little bit more flexible code which is otherwise identical to your update.

@TheSkyentist
Copy link
Contributor Author

Oh I think only write-access members of a repository can see review requests, I'm not seeing anything popping up on my end. Perhaps I'm looking in the wrong place?

@gbrammer
Copy link
Owner

That's strange. I would have thought you could see the code review as the person who originally filed the PR. I edited the file myself with the requested change.

@gbrammer gbrammer merged commit 73bb3ee into gbrammer:master Dec 29, 2023
9 checks passed
@TheSkyentist TheSkyentist deleted the filter-combo-headers branch February 29, 2024 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants