ywangd / pybufrkit

Pure Python toolkit to work with WMO BUFR messages
http://pybufrkit.readthedocs.io/
MIT License
72 stars 28 forks source link

IndexError on NUMERIC_MISSING_VALUES #20

Open turnbullerin opened 1 year ago

turnbullerin commented 1 year ago

In working with BUFR data from SIO Lagrangian Drifter Lab, I found some data where nbits=38 which caused an IndexError in this section of bitops.py as the number of bits only goes up to 32:

    def read_uint_or_none(self, nbits):
        value = self.read_uint(nbits)
        if nbits > 1 and value == NUMERIC_MISSING_VALUES[nbits]:
            value = None
        return value

To fix this, I'd like to submit a pull request that will raise the valid nbits to 64 as a quick fix in constants.py. As a more robust option, is it worth replacing this with a function that can handle the higher options (and perhaps use the constant for quick computation of the value)? something like

def is_missing_value(nbits, value):
    if nbits < 33:
        return value == NUMERIC_MISSING_VALUES[nbits]
    else:
        return value == 2 ** (nbits - 1)

If so, I can work on a patch for that too.

ywangd commented 1 year ago

I am not aware of any numeric entry takes more than 32 bits. It's possible in theory (the spec does not seem to forbid it). But I haven't seen one in practice. Do you mind share the descriptor of this element? Is it from a local table?

Having it checked dynamically as you suggested can be useful. This plus your PR #21 that bumps it to handle 64 bit should give enough coverage.

PS: this may be a typo in your code snippet, but it should be

return value == 2 ** nbits - 1

Note the removal of parathesis.