fix(analysis): break ties between number readings by the sheet's evidence, and flag disagreement

docs/concepts.md

@@ -41,6 +41,35 @@ Why that matters, from a real file: a column of `1,00 2,00 3,00` yields a decima
 confidence. That is arithmetically perfect and practically useless — the column is an identifier, and
 only the facts beside it (every value distinct) say so.
 
+## Which columns a sheet has
+
+A sheet's columns reach as far as its last value, in the header row or below it. A row's empty cells
+past its last value are padding — a workbook writes cells for their formatting alone, a csv line can
+end in delimiters — and make no column. A value anywhere does: a note typed to the right of the
+header row, or a value in a column the header does not name, makes a column with an empty `Header`,
+and it keeps its index. A consumer that refuses empty headers refuses such a sheet, so it should
+decide what an unnamed column that carries data means to it.
+
+## When readings tie
+
+Some values read completely under two cultures as different things. `11.01.2018` is 11 January in
+German and 1 November in American; `48.137` is a decimal in English and the grouped integer 48137 in
+German. The hypotheses then rank the reading the evidence favours first, and the extremes in the
+facts follow it:
+
+- **dates** go to the culture whose date separator the values are written with;
+- **numbers** go to the decimal separator the sheet's other, unambiguous columns use, then the one a
+  csv's delimiter implies (`;` a comma, `,` a point), and with nothing to go on to the decimal rather
+  than the grouped integer.
+
+That is a preference, not evidence, and the facts say so: `DateReadingsDisagree` and
+`NumberReadingsDisagree` are true when the cultures that read the most values read some row
+differently. A screen can then ask instead of guessing.
+
+A group, by the grouping rule both halves share, is three digits after a group separator; the group
+before the first separator may be any length. So under German `1234.567` reads as 1234567, and
+`12.34` is no number at all.
+
 ## What belongs to the sheet
 
 Each `SheetProfile` says what its own source was: `Format`, `Source` (a path inside an archive, else

src/TriasDev.Tabular/Analysis/ColumnFacts.cs

@@ -95,6 +95,18 @@ public sealed record ColumnFacts
     /// </remarks>
     public bool DateReadingsDisagree { get; init; }
 
+    /// <summary>
+    /// Whether the cultures that read the most values as numbers read some row as different numbers.
+    /// </summary>
+    /// <remarks>
+    /// True for a column whose every value has three digits after one separator, <c>48.137</c>: a
+    /// decimal under English, the grouped integer 48137 under German, and every value fits both. The
+    /// hypotheses rank the reading the evidence favours first — the sheet's other columns, then a
+    /// csv's delimiter, then the decimal — and the extremes in these facts follow it, but that is a
+    /// preference, not proof; this says so, so a screen can ask rather than guess (#61).
+    /// </remarks>
+    public bool NumberReadingsDisagree { get; init; }
+
     /// <summary>
     /// A bounded sample of values with how often each was seen.
     /// </summary>

src/TriasDev.Tabular/Analysis/ColumnProfiler.cs

@@ -1,4 +1,5 @@
 using System.Globalization;
+using System.Runtime.CompilerServices;
 
 namespace TriasDev.Tabular;
 
@@ -165,12 +166,13 @@ public void Accept(in RawCell cell, int rowNumber)
         bool couldBeNumeric = CultureAccumulator.CouldBeNumeric(text);
         bool couldBeDate = DateReading.LooksLikeOne(text);
 
+
         for (int i = 0; i < _cultures.Length; i++)
         {
             int twin = _numberTwin[i];
 
             _numberReads[i] = twin == i ? _cultures[i].ReadNumber(text, couldBeNumeric) : _numberReads[twin];
-            _cultures[i].AcceptText(text, rowNumber, _numberReads[i], couldBeNumeric, couldBeDate);
+            _cultures[i].AcceptText(text, rowNumber, _numberReads[i], couldBeNumeric, couldBeDate, fingerprintNumbers: twin == i);
         }
     }
 
@@ -186,9 +188,9 @@ private enum NumberKind : byte
     private readonly record struct NumberRead(NumberKind Kind, decimal Value);
 
     /// <summary>Renders what has been measured so far.</summary>
-    public ColumnFacts ToFacts()
+    public ColumnFacts ToFacts(string? preferredDecimalSeparator = null)
     {
-        CultureAccumulator best = BestCulture();
+        CultureAccumulator best = BestCulture(preferredDecimalSeparator);
 
         return new ColumnFacts
         {
@@ -209,6 +211,8 @@ public ColumnFacts ToFacts()
             DistinctCountIsExact = _distinctIsExact,
             IsUnique = Unique(),
             DateReadingsDisagree = DateReadingsDisagree(),
+            // Twins read numbers alike by construction and carry no fingerprint of their own.
+            NumberReadingsDisagree = ReadingsDisagree(c => c.NumericCount, c => c.NumberFingerprint, numbersOnly: true),
             DistinctSamples = TopFrequencies(),
             Samples = [.. _firstValues],
             DistinctValues = _distinctValuesComplete ? [.. _distinctValues] : [],
@@ -223,12 +227,22 @@ public ColumnFacts ToFacts()
     /// Only used to pick which culture's extremes to publish. It is not a verdict on the column, and
     /// the per-culture counts stay available so that a caller can disagree.
     /// </remarks>
-    private CultureAccumulator BestCulture()
+    private CultureAccumulator BestCulture(string? preferredDecimalSeparator)
     {
         CultureAccumulator best = _cultures[0];
 
         foreach (CultureAccumulator culture in _cultures)
         {
+            // #61: of cultures reading as many numbers, the one the evidence favours, so that the
+            // extremes describe the reading that ranks first rather than the first culture listed.
+            if (preferredDecimalSeparator is not null && culture.NumericCount > 0
+                && culture.NumericCount == best.NumericCount
+                && culture.DecimalSeparator == preferredDecimalSeparator && best.DecimalSeparator != preferredDecimalSeparator)
+            {
+                best = culture;
+                continue;
+            }
+
             // The third criterion is #58: of cultures reading as many dates, the one whose separator the
             // dates are written with, so 11.01.2018 publishes January rather than November.
             if (culture.NumericCount > best.NumericCount
@@ -246,9 +260,12 @@ private CultureAccumulator BestCulture()
     /// <summary>
     /// Whether two cultures that read the most dates read some row as different dates.
     /// </summary>
-    private bool DateReadingsDisagree()
+    private bool DateReadingsDisagree() => ReadingsDisagree(c => c.DateCount, c => c.DateFingerprint, numbersOnly: false);
+
+    /// <summary>Whether two cultures that read the most values of a kind read some row differently.</summary>
+    private bool ReadingsDisagree(Func<CultureAccumulator, int> count, Func<CultureAccumulator, ulong> fingerprint, bool numbersOnly)
     {
-        int most = _cultures.Max(c => c.DateCount);
+        int most = _cultures.Max(count);
 
         if (most == 0)
         {
@@ -257,18 +274,20 @@ private bool DateReadingsDisagree()
 
         ulong? first = null;
 
-        foreach (CultureAccumulator culture in _cultures)
+        for (int i = 0; i < _cultures.Length; i++)
         {
-            if (culture.DateCount != most)
+            CultureAccumulator culture = _cultures[i];
+
+            if (count(culture) != most || (numbersOnly && _numberTwin[i] != i))
             {
                 continue;
             }
 
             if (first is null)
             {
-                first = culture.DateFingerprint;
+                first = fingerprint(culture);
             }
-            else if (culture.DateFingerprint != first)
+            else if (fingerprint(culture) != first)
             {
                 return true;
             }
@@ -277,6 +296,30 @@ private bool DateReadingsDisagree()
         return false;
     }
 
+    /// <summary>
+    /// How strongly this column's own values speak for a decimal separator: the most numbers a
+    /// comma culture read, less the most a point culture read. Zero where both read alike.
+    /// </summary>
+    internal int DecimalCommaEvidence()
+    {
+        int comma = 0;
+        int point = 0;
+
+        foreach (CultureAccumulator culture in _cultures)
+        {
+            if (culture.DecimalSeparator == ",")
+            {
+                comma = Math.Max(comma, culture.NumericCount);
+            }
+            else if (culture.DecimalSeparator == ".")
+            {
+                point = Math.Max(point, culture.NumericCount);
+            }
+        }
+
+        return comma - point;
+    }
+
     /// <summary>
     /// Whether this column can identify its rows.
     /// </summary>
@@ -454,10 +497,29 @@ private static char DateSeparatorOf(string name)
         /// <summary>An order-free sum over the rows of which date each read as.</summary>
         public ulong DateFingerprint { get; private set; }
 
-        private static ulong RowDateHash(int rowNumber, DateTime date)
+        /// <summary>An order-free sum over the rows of which number each read as.</summary>
+        public ulong NumberFingerprint { get; private set; }
+
+        /// <summary>The decimal separator this culture reads numbers with.</summary>
+        public string DecimalSeparator => _culture.NumberFormat.NumberDecimalSeparator;
+
+        private void FingerprintNumber(int rowNumber, decimal value, bool fingerprint)
+        {
+            if (!fingerprint)
+            {
+                return;
+            }
+
+            // The value's sixteen bytes as they are: GetHashCode normalises the scale first, and GetBits
+            // copies them out, each costing more than the hash.
+            ref ulong low = ref Unsafe.As<decimal, ulong>(ref value);
+            NumberFingerprint += RowHash(rowNumber, (long)(low ^ (Unsafe.Add(ref low, 1) * 0x9E3779B97F4A7C15UL)));
+        }
+
+        private static ulong RowHash(int rowNumber, long value)
         {
             // SplitMix64 over the pair, so that two different readings of a row cannot cancel out.
-            ulong x = ((ulong)(uint)rowNumber << 40) ^ (ulong)date.Ticks;
+            ulong x = ((ulong)(uint)rowNumber << 40) ^ (ulong)value;
             x = (x ^ (x >> 30)) * 0xBF58476D1CE4E5B9UL;
             x = (x ^ (x >> 27)) * 0x94D049BB133111EBUL;
             return x ^ (x >> 31);
@@ -551,18 +613,24 @@ public NumberRead ReadNumber(string text, bool couldBeNumeric)
         /// <param name="rowNumber">Where it stands, for an outlier.</param>
         /// <param name="number">What <see cref="ReadNumber"/> made of it, here or under a twin culture.</param>
         /// <param name="couldBeDate">Whether the value has the shape of a date, asked once by the caller.</param>
-        public void AcceptText(string text, int rowNumber, NumberRead number, bool couldBeNumeric, bool couldBeDate)
+        /// <param name="fingerprintNumbers">
+        /// False for a culture that reads numbers as an earlier one does: its readings are that one's,
+        /// so hashing them again would only cost time.
+        /// </param>
+        public void AcceptText(string text, int rowNumber, NumberRead number, bool couldBeNumeric, bool couldBeDate, bool fingerprintNumbers)
         {
             switch (number.Kind)
             {
                 case NumberKind.Integer:
                     _integer++;
                     Widen(number.Value);
+                    FingerprintNumber(rowNumber, number.Value, fingerprintNumbers);
                     break;
 
                 case NumberKind.Decimal:
                     _decimal++;
                     Widen(number.Value);
+                    FingerprintNumber(rowNumber, number.Value, fingerprintNumbers);
                     break;
 
                 case NumberKind.None:
@@ -592,7 +660,7 @@ public void AcceptText(string text, int rowNumber, NumberRead number, bool could
                 // Which date each row read as, summed so the order of rows does not matter: two
                 // cultures that read every row alike end equal, and one row read otherwise sets them
                 // apart — day and month swapped included.
-                DateFingerprint += RowDateHash(rowNumber, date);
+                DateFingerprint += RowHash(rowNumber, date.Ticks);
             }
             else if (_dateOutliers.Count < outlierLimit)
             {

src/TriasDev.Tabular/Analysis/CultureCatalog.cs

@@ -39,6 +39,10 @@ public static bool TryGet(string? name, out CultureInfo culture)
         }
     }
 
+    /// <summary>The decimal separator a culture reads numbers with, or null for one that is not available.</summary>
+    public static string? DecimalSeparatorOf(string? name) =>
+        TryGet(name, out CultureInfo culture) ? culture.NumberFormat.NumberDecimalSeparator : null;
+
     /// <summary>The names this runtime has, in order; the invariant culture when it has none of them.</summary>
     public static IReadOnlyList<string> Available(IReadOnlyList<string> names)
     {

src/TriasDev.Tabular/Analysis/HypothesisBuilder.cs

@@ -21,7 +21,7 @@ internal static class HypothesisBuilder
     /// <param name="minimumConfidence">
     /// The share of values a reading must account for before it is offered. Text is exempt.
     /// </param>
-    public static IReadOnlyList<TypeHypothesis> Build(ColumnFacts facts, double minimumConfidence = 0)
+    public static IReadOnlyList<TypeHypothesis> Build(ColumnFacts facts, double minimumConfidence = 0, string? preferredDecimalSeparator = null)
     {
         ArgumentNullException.ThrowIfNull(facts);
 
@@ -59,7 +59,8 @@ public static IReadOnlyList<TypeHypothesis> Build(ColumnFacts facts, double mini
         return [.. hypotheses
             .OrderBy(h => h.Type == ColumnType.Text ? 1 : 0)
             .ThenByDescending(h => h.Confidence)
-            .ThenByDescending(h => Specificity(h.Type))
+            .ThenByDescending(h => Favoured(facts, h, preferredDecimalSeparator))
+            .ThenByDescending(h => Specificity(h.Type, facts.NumberReadingsDisagree))
             .ThenByDescending(h => OwnSeparatorDates(facts, h))
             .ThenBy(h => h.Culture, StringComparer.Ordinal)];
     }
@@ -163,13 +164,28 @@ private static void Add(
     /// <summary>
     /// How much a reading claims. Used only to order equally confident ones, narrowest first.
     /// </summary>
-    private static int Specificity(ColumnType type) =>
+    /// <summary>
+    /// Where numeric readings disagree, whether this one reads numbers with the separator the evidence
+    /// favours (#61). Where they agree it decides nothing, and the order stays what it was.
+    /// </summary>
+    private static int Favoured(ColumnFacts facts, TypeHypothesis hypothesis, string? preferredDecimalSeparator) =>
+        facts.NumberReadingsDisagree && preferredDecimalSeparator is not null
+            && hypothesis.Type is ColumnType.Integer or ColumnType.Decimal
+            && CultureCatalog.DecimalSeparatorOf(hypothesis.Culture) == preferredDecimalSeparator
+                ? 1
+                : 0;
+
+    /// <summary>
+    /// The narrower reading first — except that where numeric readings disagree a decimal goes before
+    /// an integer, as a grouped integer rests on one group, which is weak evidence of grouping.
+    /// </summary>
+    private static int Specificity(ColumnType type, bool numbersDisagree) =>
         type switch
         {
             ColumnType.Boolean => 5,
             ColumnType.Date => 4,
-            ColumnType.Integer => 3,
-            ColumnType.Decimal => 2,
+            ColumnType.Integer => numbersDisagree ? 2 : 3,
+            ColumnType.Decimal => numbersDisagree ? 3 : 2,
             _ => 0,
         };
 }

src/TriasDev.Tabular/Analysis/TabularAnalyzer.cs

@@ -202,7 +202,7 @@ private SheetProfile AnalyzeSheet(
             reporter.Row(sheet);
         }
 
-        SheetProfile profile = BuildProfile(sheet, rowCount, profilers);
+        SheetProfile profile = BuildProfile(sheet, rowCount, profilers, cursor.Dialect?.Delimiter);
 
         foreach (ColumnProfiler profiler in profilers)
         {
@@ -242,17 +242,18 @@ private void AcceptRow(
         }
     }
 
-    private SheetProfile BuildProfile(SheetInfo sheet, int rowCount, List<ColumnProfiler> profilers)
+    private SheetProfile BuildProfile(SheetInfo sheet, int rowCount, List<ColumnProfiler> profilers, char? delimiter)
     {
         List<ColumnProfile> columns = [];
+        string? separator = PreferredDecimalSeparator(profilers, delimiter);
 
         foreach (ColumnProfiler profiler in profilers)
         {
-            ColumnFacts facts = profiler.ToFacts();
+            ColumnFacts facts = profiler.ToFacts(separator);
             columns.Add(new ColumnProfile
             {
                 Facts = facts,
-                Hypotheses = HypothesisBuilder.Build(facts, _options.MinimumHypothesisConfidence),
+                Hypotheses = HypothesisBuilder.Build(facts, _options.MinimumHypothesisConfidence, separator),
             });
         }
 
@@ -270,6 +271,33 @@ private SheetProfile BuildProfile(SheetInfo sheet, int rowCount, List<ColumnProf
         };
     }
 
+    /// <summary>
+    /// The decimal separator the sheet's evidence favours, for columns whose values read completely
+    /// under both — <c>48.137</c>, a decimal or a grouped integer (#61) — or null when nothing does.
+    /// </summary>
+    /// <remarks>
+    /// First the sheet's other columns: one where a comma culture reads more numbers than a point
+    /// culture, or the other way round, is not in doubt, and a sheet uses one convention. Then a
+    /// csv's delimiter: a comma-delimited file cannot use an unquoted comma for decimals, and a
+    /// semicolon-delimited one is German as a rule.
+    /// </remarks>
+    private static string? PreferredDecimalSeparator(List<ColumnProfiler> profilers, char? delimiter)
+    {
+        int votes = profilers.Sum(p => Math.Sign(p.DecimalCommaEvidence()));
+
+        return votes switch
+        {
+            > 0 => ",",
+            < 0 => ".",
+            _ => delimiter switch
+            {
+                ';' => ",",
+                ',' => ".",
+                _ => null,
+            },
+        };
+    }
+
     /// <summary>The row up to and including its last value; empty when it holds none.</summary>
     private static ReadOnlySpan<RawCell> ToLastValue(ReadOnlySpan<RawCell> row)
     {

src/TriasDev.Tabular/PublicAPI.Unshipped.txt

@@ -3,3 +3,5 @@ TriasDev.Tabular.ColumnFacts.DateReadingsDisagree.get -> bool
 TriasDev.Tabular.ColumnFacts.DateReadingsDisagree.init -> void
 TriasDev.Tabular.CultureParseCounts.DatesWithOwnSeparator.get -> int
 TriasDev.Tabular.CultureParseCounts.DatesWithOwnSeparator.init -> void
+TriasDev.Tabular.ColumnFacts.NumberReadingsDisagree.get -> bool
+TriasDev.Tabular.ColumnFacts.NumberReadingsDisagree.init -> void