deeptools / pyBigWig

A python extension for quick access to bigWig and bigBed files
MIT License
216 stars 48 forks source link

pyBwGetChroms does not work any more with a BigWig that has headers #97

Closed blaiseli closed 5 years ago

blaiseli commented 5 years ago

I have code using pyBigWig that used to work, but it seems that it doesn't work any more with newer versions. I get the following error message:

Chromosomes cannot be accessed in files opened for writing!

This is likely due to this piece of code:

https://github.com/deeptools/pyBigWig/blob/master/pyBigWig.c#L329

    if(bw->isWrite == 1) {
        PyErr_SetString(PyExc_RuntimeError, "Chromosomes cannot be accessed in files opened for writing!");
        return NULL;
    }

I see that this was not present as of commit c514a8a68e7500dc9156552d16e6ae311f559760, for instance.

My code is as follows:

                bw_out = pyBigWig.open(output.bw, "w")
                bw_out.addHeader(list(chrom_sizes.items()))
                for (chrom, chrom_len) in bw_out.chroms().items():

If I guess correctly, it was OK to access chromosomes once the header was added. Is there a reason why this is not the case any more ?

dpryan79 commented 5 years ago

I don't think I ever supported reading files opened for writing, actually. You probably changed your code from:

from chrom, chrom_len in chrom_sizes.items():
blaiseli commented 5 years ago

I can easily change the code as you suggest, its not really a problem.

That said, I'm quite sure the current piece of code works with older versions of pyBigWig. I have several snakemake workflows that use is, on a workstation where pyBigWig.__version__ says '0.3.12' and neither me nor my colleagues observed this error.

On that workstation, this indeed seems possible:

>>> import pyBigWig
>>> pyBigWig.__version__
'0.3.12'
>>> chrom_sizes = {"A": 100, "B": 150}
>>> bw_out = pyBigWig.open("/tmp/test.bw", "w")
>>> bw_out.addHeader(list(chrom_sizes.items()))
>>> bw_out.addHeader(list(chrom_sizes.items()))
>>> for (chrom, chrom_len) in bw_out.chroms().items():
...     print(chrom, chrom_len)
... 
A 100
B 150

I observed the error while testing one of my workflows in a container where I installed the latest versions of python packages.

dpryan79 commented 5 years ago

Ah, now I remember, this was disabled to fix #88. I'd prefer to keep the current behavior rather than allowing this, since otherwise people are likely to try and read headers before adding them.

blaiseli commented 5 years ago

That's fine. Thanks for the explanations.