Skip to content

Conversation

@dmalzl
Copy link
Contributor

@dmalzl dmalzl commented Jul 7, 2025

This is a fix for #556

@Phlya
Copy link
Member

Phlya commented Jul 7, 2025

Nice, thank you! Would it be possible add a simple test to ensure the dtypes are propagated?

@dmalzl
Copy link
Contributor Author

dmalzl commented Jul 7, 2025

Yes. I'll try :)

@Phlya
Copy link
Member

Phlya commented Jul 7, 2025

It looks good, but the test actually fails... Does it work for you locally?

@dmalzl
Copy link
Contributor Author

dmalzl commented Jul 7, 2025

Yes, also just noticed. Was a premature push. i did not think about the rearranged pixels after calling rearrange_cooler. Currently trying to figure out if I can just bypass the order check in some elegant way.

@dmalzl
Copy link
Contributor Author

dmalzl commented Jul 7, 2025

Fixed it by simply leaving the chromosome order as is and just checking if dtypes are equal. This now passes locally and I also fails when dtypes are not equal

assert np.array_equal(old_trans_m, reordered_inverted_trans_m, equal_nan=True)

# IV.
# Check that pixel datatypes are propagted to output cooler
Copy link
Member

Choose a reason for hiding this comment

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

propagAted

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for pointing this out. already fixed it

@dmalzl
Copy link
Contributor Author

dmalzl commented Jul 7, 2025

Thanks for the review and sorry for the inability to type straight^^

@Phlya
Copy link
Member

Phlya commented Jul 7, 2025

haha no worries! Thank you for the PR, I'll merge it!

@Phlya Phlya merged commit b1911f7 into open2c:master Jul 7, 2025
1 check passed
@dmalzl
Copy link
Contributor Author

dmalzl commented Jul 7, 2025

Great :) Always happy to help making the ecosystem better ;). Have a nice day

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