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-38372: Switch to numpy random generator for test_catalogMatch and add seed #74

Merged
merged 1 commit into from Mar 21, 2023

Conversation

cmsaunders
Copy link
Contributor

No description provided.

Copy link
Contributor

@natelust natelust left a comment

Choose a reason for hiding this comment

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

See my one comment. Also since you switched to the new style of an instance generator, can you squash away the commit setting the seed?

@@ -32,6 +32,8 @@
from lsst.daf.butler import DatasetRef, DatasetType, DimensionUniverse, StorageClass
from lsst.meas.algorithms import ReferenceObjectLoader

rng = np.random.default_rng(12345)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you are no longer relying on modifying global numpy random behavior and are using an instance of a random generator, can you move this to be an instance variable of the class you use it in?

@cmsaunders
Copy link
Contributor Author

Thanks! Yes, I'll squash. I should have said that I was planning to squash once the review was done.

@cmsaunders cmsaunders merged commit 7a12b10 into main Mar 21, 2023
7 checks passed
@cmsaunders cmsaunders deleted the tickets/DM-38372 branch March 21, 2023 00:15
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.

None yet

2 participants