aidenlab / straw

Extract data quickly from Juicebox via straw
MIT License
62 stars 36 forks source link

Fixes for Windows #69

Closed cwenger closed 3 years ago

cwenger commented 3 years ago

Main changes:

Also a few minor fixes for compiling with Microsoft Visual Studio.

sa501428 commented 3 years ago

I do prefer the idea of defining the lengths, especially as they are technically decided by the hic file format, not the length of particular variable types.

Other major issue is that we actually have a pretty substantial restructuring of straw to better handle more advanced use cases: https://github.com/sa501428/straw Can you actually make the pull request to that repo. After we finish up some testing, that fork will get merged here and overwrite the master branch here.

cwenger commented 3 years ago

I think you did a search and replace, line 88 is straw-R.cpp now has a typo.

Yes good catch, I fixed that in the native C++ code but not the R C++ code! Corrected now.

sa501428 commented 3 years ago

@cwenger please see the changes pulled into the master branch. There is a lot of restructuring internally. Old usage is still supported, but new usage has also been added.

cwenger commented 3 years ago

Thanks @sa501428, will do!

sa501428 commented 3 years ago

Just posting this reference here for the definitions used in the hic file format

Strings are null (0) terminated. So for example the string "HIC" is represented by 4 bytes [48 49 43 0] short - 16 bit integer int - 32 bit integer long - 64 bit integer float - 32 bit floating point double - 64 bit floating point

sa501428 commented 3 years ago

Actually I'm confused now - how does this function still work on Windows?

long long readLongLongFromFile(istream &fin) {
    long long tempLong;
    fin.read((char *) &tempLong, sizeof(long));
    return tempLong;
}

Given it still ends up calling sizeof(long) and not sizeof(long long)? Doesn't that suggest the size of long is fine?

cwenger commented 3 years ago

@sa501428 You are correct. I missed a couple longs when I was re-implementing the changes, and I only tested in Windows with R, not with the command-line version. I'll do another PR to fix this.