-
Notifications
You must be signed in to change notification settings - Fork 50
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
New tool: rearrange cooler #448
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Thank you for the idea @golobor, mapping bin IDs from old to new instead of complicated arithmetic is way simpler! I don't think there is a big difference in performance. |
@golobor any comments now? Any ideas how to incorporate the notebook from the sandbox into the docs, maybe, or how to improve it? |
tests/test_reorder_cooler.py
Outdated
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.
If we keep "reorder" in the sandbox, we may want to keep the tests for the sandbox separately.
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.
oh do you mean the name of the file with the tests? I just forgot to change it... Or what do you mean?
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.
If we don't include it as a main tool, maybe running this test conditionally on request is better? E.g. if we introduce some big change that affects everything, we don't need to debug the sandbox in the first place.
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.
But it's moved to the main tools in this PR, I don't see any reason not to do it. Why not just run this test every time?
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.
Ah, if you think it's ready for the main tools, then no change is needed, of course.
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 hope it's ready or nearly ready! I think we could move the example notebook somewhere more discoverable, otherwise it seems to work ax expected.
Maybe one more thing is to check the case when view regions are not bin-aligned...
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.
My tests are running perfectly, and the tool looks ready to become part of the main cooltools. If we don't do that for now, we should somehow separate tests for the sandbox from the main ones.
@nvictus I now specify mergebuf here because I had issues with truncated files with the warning about it, as I mentioned at some point. Do you think it's OK like this? Or should it be an argument? |
@agalitsyna @golobor it's been a long time, but I think this can be merged. The example notebook is not very discoverable at the moment, but I think it should be expanded into a more general "how to manipulate coolers" notebook and be part of open2c_examples, and I don't think that's part of this PR... After merging we can simply move it there as is for now though. |
Hello, How to subset .mcool files in cooltools? |
Both API and (first draft of) CLI to rearrange a cooler following a genomic view (almost like liftover).
Need to move the example notebook from sandbox to the main examples at some point...