xbitinfo
xbitinfo copied to clipboard
observingClouds
Add nan warning in get_bitinformation; Closes #200
Let's start by closing the issue on warnings, but I think the implementations could be rewritten to handle NaN's, which would be good for masked datasets.
I apologize, my organization is swapping SSL certs and that seems to be preventing me from running the unit test, but this PR is straightforward.
Thanks for the PR. First time contributors cannot run the tests automatically. We allow those manually. So no issue on your side.
Ideally NaNs should be masked and then ignored in the bit pair count algorithm so that in a bitstream of 1 0 NaN 0 1 it would only count two bit pairs 10 and 01 instead of counting across the bits of the NaN (which is a valid operation as it's also just bits, but not really meaningful). This is how I've implemented it in BitInformation.jl
https://github.com/milankl/BitInformation.jl/blob/953dac3b5ae12a719456f1be3ae0ef4317f24e86/src/bit_count.jl#L127-L131
although NaN's are not automatically converted to a mask at the moment, but that could easily be added. Well maybe requires some work to avoid a double pass of the data, but yes.
Changelog updated. And I meant that the SSL was preventing me from running tests locally. I don't fully understand why, though I could see this going awry if Julia, pip, conda aren't all configured correctly...But that's for me to worry about.
Well, this is awkward. xb.get_bitinformation calls itself such the warning is thrown at least twice (+1 per additional dim). Need to ponder the simplest way to avoid that.
Eh, I don't particularly like my solution, which just loosens the unit test to permit the multiple warnings. The alternative is to import some other module that let's us inspect the calling function to decide whether or not to throw the warning. Let me know if you'd prefer that.
Well, this is awkward.
xb.get_bitinformationcalls itself such the warning is thrown at least twice (+1 per additional dim). Need to ponder the simplest way to avoid that.
Oh, that is unfortunate. Let me fix this in https://github.com/observingClouds/xbitinfo/issues/261.