-
Notifications
You must be signed in to change notification settings - Fork 0
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
Move simple simulator and model from hicat-package2 to catkit2 #192
Conversation
1db9c0f
to
501fda9
Compare
08cdc89
to
0e40700
Compare
This is ready for review, although I don't really know how to make the test make more sense, and whether it's worth it. |
tests/test_optical_model.py
Outdated
size1 = test_wf.intensity.shaped.shape[0] | ||
size2 = test_wf.intensity.shaped.shape[1] | ||
|
||
stream = DataStream.create('images', 'mod', 'float64', [size1, size2], 20) |
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 sure why the data stream is needed here. This is a leftover from very early on, when I wanted to test an early version of the camera viewer UI.
The test looks fine to me. Basic functionality test and checks that the code doesn't crash. |
d2f435e
to
68a1b62
Compare
29d5f02
to
e987286
Compare
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 overall.
Just a few small questions, nothing serious
e987286
to
a446d9d
Compare
@raphaelpclt all fair questions, I hope I answered them sufficiently, ready for a new review. Let me know if you'd need more. |
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
Fixes #187.
Pair PR with https://github.com/spacetelescope/hicat-package2/pull/587.
Needs testing and converting the added test into a real test that pytest understands.