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

DM-49274: Stop sky objects appearing in NO_DATA regions #369

Merged
merged 3 commits into from
Mar 25, 2025

Conversation

leeskelvin
Copy link
Contributor

No description provided.

@leeskelvin leeskelvin force-pushed the tickets/DM-49274 branch 3 times, most recently from eb021ba to 89727b2 Compare March 19, 2025 13:50
@@ -370,6 +370,7 @@ def __call__(self, data: KeyedData, **kwargs) -> Vector:
def setDefaults(self):
self.selectWhenFalse = [
"{band}_pixelFlags_edge",
"{band}_pixelFlags_nodata",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mind adding super().setDefaults() above this line while you're at it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added into the setDefaults for SkyObjectSelector, SkySourceSelector, and GoodDiaSourceSelector. How does that read now @arunkannawadi?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It reads great now. Thanks, Lee

@@ -392,6 +393,7 @@ def __call__(self, data: KeyedData, **kwargs) -> Vector:
def setDefaults(self):
self.selectWhenFalse = [
"pixelFlags_edge",
"pixelFlags_nodata",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly super().setDefaults() here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See previous comment.

self.assertEqual(DiaSourceCount["numAllDiaSources"].quantity.value, 558)
self.assertEqual(DiaSourceCount["numGoodDiaSources"].quantity.value, 547)
self.assertEqual(DiaSourceCount["ratioGoodToAllDiaSources"].quantity.value, 547 / 558)
self.assertEqual(DiaSourceCount["numAllDiaSources"].quantity.value, 2994)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a big change - any idea why?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes - I didn't know the data ID that was used to generate the original dataset, so I guessed my own. This means that the output catalog has grown in size from the prior 558 entries to 2994. Undoubtedly, my data ID is not the same one that was used to generate the original test sample.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK that is fair! Whoever made that original dataset should have documented it better 😜 would you consider adding the script or similar you used to generate the test data for next time?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added my workflow into this Jira ticket comment: https://rubinobs.atlassian.net/browse/DM-49274?focusedCommentId=581302.

Is that a sufficient place to document this @mrawls?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is great - my initial thought was a text file or script of some kind in the new data subdirectory, but honestly having it on the Jira ticket is at least as good, thank you

@leeskelvin leeskelvin merged commit 1c1b662 into main Mar 25, 2025
13 checks passed
@leeskelvin leeskelvin deleted the tickets/DM-49274 branch March 25, 2025 10:52
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.

3 participants