search

maximilianh / cellBrowser

main repo: https://github.com/ucscGenomeBrowser/cellBrowser/ - Python pipeline and Javascript scatter plot library for single-cell datasets, http://cellbrowser.rtfd.org
https://github.com/ucscGenomeBrowser/cellBrowser/
GNU General Public License v3.0
104 stars 40 forks source link

When reading mtx, produce float32 values, consistent with .tsv.gz. #184

Closed mxposed closed 4 years ago

mxposed commented 4 years ago

Fixes #183

maximilianh commented 4 years ago

Oh man, I was trying to fix this yesterday but couldn't find it. In the end I moved the covid-airways dataset back to .tsv.gz Thsi is so much better. Many thanks!

maximilianh commented 4 years ago

Oh and now I understand that if numpy is not installed, the bug doesn't appear. I wonder if in exprEncode(), where I run exprArr.tobytes() I shouldn't also force a 32bit float or abort if the type is not 32bit, instead of just using whatever array was supplied to the function. with Javascript, 64bit floats should never be allowed as input...

mxposed commented 4 years ago

It sounds like a good idea, ensuring type before writing it in exprEncode. And in this solution, I totally forgot about int matrices, and I expect numpy to be installed

maximilianh commented 4 years ago

So, many thanks for this change, staring more at this code made me realize how poorly written this was. I totally forgot that ints are not necessarily uint32.

I guess if this ever worked, it must have been by accident? I removed your change and instead put it into exprEncode (the function that converts a gene expression vector to a byte string). It now forces values to be either uint32 or float32. It must have used originally int64, as numpy defaults to that, which is why I don't know how it ever could have worked before with integers...

On Tue, Jul 14, 2020 at 4:47 PM Nikolay Markov notifications@github.com wrote:

It sounds like a good idea, ensuring type before writing it in exprEncode. And in this solution, I totally forgot about int matrices, and I expect numpy to be installed

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/maximilianh/cellBrowser/pull/184#issuecomment-658223747, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACL4TNKYHVANFOEIVCKZCLR3RVYRANCNFSM4OZEE3FQ .