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

Rework more tests #578

Merged
merged 4 commits into from
Nov 7, 2023
Merged

Rework more tests #578

merged 4 commits into from
Nov 7, 2023

Conversation

rpauszek
Copy link
Contributor

@rpauszek rpauszek commented Oct 3, 2023

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

@rpauszek rpauszek requested review from a team as code owners October 3, 2023 09:38
@rpauszek rpauszek requested review from dean0x7d and JoepVanlier and removed request for a team October 3, 2023 09:38
@rpauszek rpauszek force-pushed the rework_plotting_tests branch 2 times, most recently from 0d87195 to 8f62a18 Compare October 27, 2023 11:36
@rpauszek rpauszek force-pushed the rework_plotting_tests branch from 8f62a18 to 95c0306 Compare October 27, 2023 13:56
Base automatically changed from rework_plotting_tests to main October 27, 2023 14:08
Copy link
Member

@JoepVanlier JoepVanlier left a 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
Copy link
Member

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"]
Copy link
Member

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?

Comment on lines 170 to 177
# Test cropping in base pairs
kymo_bp = kymo.calibrate_to_kbp(1.000)
Copy link
Member

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.

Comment on lines 259 to 261
print(kymo.get_image("red"))
print(cropped.get_image("red"))
print(twice_cropped.get_image("red"))
Copy link
Member

Choose a reason for hiding this comment

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

left overs

@rpauszek rpauszek force-pushed the rework_more_tests branch 2 times, most recently from d8973e2 to 21eed94 Compare November 6, 2023 16:21
Copy link
Member

@JoepVanlier JoepVanlier left a comment

Choose a reason for hiding this comment

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

Nice!

@rpauszek rpauszek merged commit d6d9d36 into main Nov 7, 2023
8 checks passed
@rpauszek rpauszek deleted the rework_more_tests branch November 7, 2023 09:20
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