-
Notifications
You must be signed in to change notification settings - Fork 6
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
Rework more tests #578
Rework more tests #578
Conversation
f5c5bbd
to
196818b
Compare
0d87195
to
8f62a18
Compare
196818b
to
e3de440
Compare
8f62a18
to
95c0306
Compare
e3de440
to
a66ed27
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 great!
Found a few leftover print statements. Other than that, really only a few minor requests.
Please don't forget to squash it though.
assert sliced.get_image("red").shape == (ref_pixels, ref_lines) | ||
np.testing.assert_allclose(sliced.get_image("red").data, ref.image[:, :, 0]) | ||
|
||
factor = 2 |
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.
nitpick: num_lines
? (factor makes me think of jumping/striding)
assert sliced.get_image("red").shape == (ref_pixels, 3) | ||
np.testing.assert_allclose(sliced.get_image("red").data, ref.image[:, :3, 0]) | ||
|
||
sliced = kymo["3.5s":"14s"] |
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.
Why go back to seconds here rather than something expressed in line times and whatnot?
# Test cropping in base pairs | ||
kymo_bp = kymo.calibrate_to_kbp(1.000) |
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 think it might make sense to split the bp ones off into their own test. Also because you essentially redefine px_size
here.
print(kymo.get_image("red")) | ||
print(cropped.get_image("red")) | ||
print(twice_cropped.get_image("red")) |
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.
left overs
d8973e2
to
21eed94
Compare
21eed94
to
7733cb6
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.
Nice!
Why this PR?
Continuing from #577 , all kymo tests dealing with slicing/cropping
I left things as separate commits to hopefully make reviewing easier. Before merge, will squash with a proper commit message