Floating point rounding error in _guess_delimiter of csv module

[paulbakkerbloom]

Bug report

Bug description:

Hi! Using the csv module - when determining the delimiter in the _guess_delimiter method, it checks the "consistency" (comparison of counts of a character between rows) of a character. It does reading chunks of 10 lines.

So for the first chunk, when a ';' appears exactly e.g. 30 times on every row - the consistency is 100% - it passes straight away and ';' is recognised as the delimiter. When it appears 30 times on 9 rows and 29 times on 1 row, the consistency is 90%.

The bug is as follows: the required consistency starts at 100% and is lowered by 1% for some iterations until it reaches the minimum threshold of 90%. Only due to floating point rounding errors the consistency becomes 0.8999999, therefore missing the last iteration.

            consistency = 1.0
            # minimum consistency threshold
            threshold = 0.9
            while len(delims) == 0 and consistency >= threshold:
                for k, v in modeList:
                    if v[0] > 0 and v[1] > 0:
                        if ((v[1]/total) >= consistency and
                            (delimiters is None or k in delimiters)):
                            delims[k] = v
                consistency -= 0.01

Image with "proof":

Whole method:

    def _guess_delimiter(self, data, delimiters):
        """
        The delimiter /should/ occur the same number of times on
        each row. However, due to malformed data, it may not. We don't want
        an all or nothing approach, so we allow for small variations in this
        number.
          1) build a table of the frequency of each character on every line.
          2) build a table of frequencies of this frequency (meta-frequency?),
             e.g.  'x occurred 5 times in 10 rows, 6 times in 1000 rows,
             7 times in 2 rows'
          3) use the mode of the meta-frequency to determine the /expected/
             frequency for that character
          4) find out how often the character actually meets that goal
          5) the character that best meets its goal is the delimiter
        For performance reasons, the data is evaluated in chunks, so it can
        try and evaluate the smallest portion of the data possible, evaluating
        additional chunks as necessary.
        """

        data = list(filter(None, data.split('\n')))

        ascii = [chr(c) for c in range(127)] # 7-bit ASCII

        # build frequency tables
        chunkLength = min(10, len(data))
        iteration = 0
        charFrequency = {}
        modes = {}
        delims = {}
        start, end = 0, chunkLength
        while start < len(data):
            iteration += 1
            for line in data[start:end]:
                for char in ascii:
                    metaFrequency = charFrequency.get(char, {})
                    # must count even if frequency is 0
                    freq = line.count(char)
                    # value is the mode
                    metaFrequency[freq] = metaFrequency.get(freq, 0) + 1
                    charFrequency[char] = metaFrequency

            for char in charFrequency.keys():
                items = list(charFrequency[char].items())
                if len(items) == 1 and items[0][0] == 0:
                    continue
                # get the mode of the frequencies
                if len(items) > 1:
                    modes[char] = max(items, key=lambda x: x[1])
                    # adjust the mode - subtract the sum of all
                    # other frequencies
                    items.remove(modes[char])
                    modes[char] = (modes[char][0], modes[char][1]
                                   - sum(item[1] for item in items))
                else:
                    modes[char] = items[0]

            # build a list of possible delimiters
            modeList = modes.items()
            total = float(min(chunkLength * iteration, len(data)))
            # (rows of consistent data) / (number of rows) = 100%
            consistency = 1.0
            # minimum consistency threshold
            threshold = 0.9
            while len(delims) == 0 and consistency >= threshold:
                for k, v in modeList:
                    if v[0] > 0 and v[1] > 0:
                        if ((v[1]/total) >= consistency and
                            (delimiters is None or k in delimiters)):
                            delims[k] = v
                consistency -= 0.01

            if len(delims) == 1:
                delim = list(delims.keys())[0]
                skipinitialspace = (data[0].count(delim) ==
                                    data[0].count("%c " % delim))
                return (delim, skipinitialspace)

            # analyze another chunkLength lines
            start = end
            end += chunkLength

        if not delims:
            return ('', 0)

        # if there's more than one, fall back to a 'preferred' list
        if len(delims) > 1:
            for d in self.preferred:
                if d in delims.keys():
                    skipinitialspace = (data[0].count(d) ==
                                        data[0].count("%c " % d))
                    return (d, skipinitialspace)

        # nothing else indicates a preference, pick the character that
        # dominates(?)
        items = [(v,k) for (k,v) in delims.items()]
        items.sort()
        delim = items[-1][1]

        skipinitialspace = (data[0].count(delim) ==
                            data[0].count("%c " % delim))
        return (delim, skipinitialspace)

CPython versions tested on:

3.10

Operating systems tested on:

macOS

[rhettinger]

Thanks for the bug report. You've done a good analysis of the behavior.

We could use exact fractional arithmetic in this section. However, the threshold of 90% was arbitrary so it likely doesn't make sense to make accumulation more exact. Also, we should be cautious about making any changes to guessing or sniffing logic. Code that is currently working but is on the margins may stop working.

[paulbakkerbloom]

Hi! Well for what it's worth as you can see from the screenshot it also already happens at 93%.

And even though I get that the threshold is arbitrary - now that it's set I do think it has value to stick to it. Simply because than the code acts the way you expect when you read it. It took me some time to figure out why this was going wrong.

If you don't want to change anything to the current logic because it might break existing stuff (not sure if I agree with this), maybe at least we should make this explicit and change to > instead of >=