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

Add ability to read bw files and return maximum value across an interval #67

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from

Conversation

donaldcampbelljr
Copy link
Member

  • created simple wrapper around BigTools BigWigRead::open_file(bigwigpath)?
    • might be unnecessary but thought it might be appreciated for making python bindings
  • created func to give max value over an interval given a chromosome name (to be used downstream in geniml)
  • added test to show it in action

@donaldcampbelljr
Copy link
Member Author

  • Needs python bindings

Copy link
Member

@nleroy917 nleroy917 left a comment

Choose a reason for hiding this comment

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

Returning an error instead of panicing would be helpful otherwise this looks great.

gtars/src/uniwig/utils.rs Outdated Show resolved Hide resolved
gtars/src/uniwig/utils.rs Outdated Show resolved Hide resolved
Copy link
Member

@nleroy917 nleroy917 left a comment

Choose a reason for hiding this comment

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

Sweet!

@nleroy917
Copy link
Member

I'll add python bindings and push here in a second

@nleroy917
Copy link
Member

nleroy917 commented Jan 15, 2025

@donaldcampbelljr I got the bindings in! Usage, then a couple notes:

import gtars

coverage = gtars.utils.read_big_wig_to_coverage("tests/data/test_all_core.bw")
val = coverage.stats("chr1", 0, 29, stat_type="max")

assert val == 4.0 # True

I have two notes on this:
I tried to keep it all as similar to what is there already, however:

  1. I changed the API to be stat_type instead of type since the latter is a reserved keyword in Rust, and
  2. To make the pyo3 bindings happy, I am doing something odd with the PyCoverage class... I simply store the file path in a string and then open it when I need to read a value. This way all the bigtools stuff stays in Rust and isn't exposed to python. This made it all super simple to write. Does this seem ok? Take a look at the commit, I am only slightly concerned with opening file over and over again.

@donaldcampbelljr
Copy link
Member Author

This PR is probably no longer necessary since the bigtools python bindings may be sufficient. We can close this once confirmed.

@nleroy917
Copy link
Member

I agree, we can close this

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.

3 participants