From 6c1cc227a52982848dfcf553bdc0ad7a2cb2c6d6 Mon Sep 17 00:00:00 2001 From: Slava Date: Tue, 29 Sep 2026 09:31:30 +0200 Subject: [PATCH] fix(analysis): numbers that read under both separators rank the one the sheet's evidence favours, and the facts say when number readings disagree --- docs/concepts.md | 29 +++++ src/TriasDev.Tabular/Analysis/ColumnFacts.cs | 12 ++ .../Analysis/ColumnProfiler.cs | 96 +++++++++++++--- .../Analysis/CultureCatalog.cs | 4 + .../Analysis/HypothesisBuilder.cs | 26 ++++- .../Analysis/TabularAnalyzer.cs | 36 +++++- src/TriasDev.Tabular/PublicAPI.Unshipped.txt | 2 + .../Analysis/AmbiguousNumberTests.cs | 104 ++++++++++++++++++ 8 files changed, 286 insertions(+), 23 deletions(-) create mode 100644 tests/TriasDev.Tabular.Tests/Analysis/AmbiguousNumberTests.cs diff --git a/docs/concepts.md b/docs/concepts.md index 29200e3..4dd6bdc 100644 --- a/docs/concepts.md +++ b/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 diff --git a/src/TriasDev.Tabular/Analysis/ColumnFacts.cs b/src/TriasDev.Tabular/Analysis/ColumnFacts.cs index 89e56c1..3083ad8 100644 --- a/src/TriasDev.Tabular/Analysis/ColumnFacts.cs +++ b/src/TriasDev.Tabular/Analysis/ColumnFacts.cs @@ -95,6 +95,18 @@ public sealed record ColumnFacts /// public bool DateReadingsDisagree { get; init; } + /// + /// Whether the cultures that read the most values as numbers read some row as different numbers. + /// + /// + /// True for a column whose every value has three digits after one separator, 48.137: 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). + /// + public bool NumberReadingsDisagree { get; init; } + /// /// A bounded sample of values with how often each was seen. /// diff --git a/src/TriasDev.Tabular/Analysis/ColumnProfiler.cs b/src/TriasDev.Tabular/Analysis/ColumnProfiler.cs index 22d8f54..a781ffa 100644 --- a/src/TriasDev.Tabular/Analysis/ColumnProfiler.cs +++ b/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); /// Renders what has been measured so far. - 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. /// - 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() /// /// Whether two cultures that read the most dates read some row as different dates. /// - private bool DateReadingsDisagree() + private bool DateReadingsDisagree() => ReadingsDisagree(c => c.DateCount, c => c.DateFingerprint, numbersOnly: false); + + /// Whether two cultures that read the most values of a kind read some row differently. + private bool ReadingsDisagree(Func count, Func 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; } + /// + /// 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. + /// + 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; + } + /// /// Whether this column can identify its rows. /// @@ -454,10 +497,29 @@ private static char DateSeparatorOf(string name) /// An order-free sum over the rows of which date each read as. public ulong DateFingerprint { get; private set; } - private static ulong RowDateHash(int rowNumber, DateTime date) + /// An order-free sum over the rows of which number each read as. + public ulong NumberFingerprint { get; private set; } + + /// The decimal separator this culture reads numbers with. + 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(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) /// Where it stands, for an outlier. /// What made of it, here or under a twin culture. /// Whether the value has the shape of a date, asked once by the caller. - public void AcceptText(string text, int rowNumber, NumberRead number, bool couldBeNumeric, bool couldBeDate) + /// + /// 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. + /// + 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) { diff --git a/src/TriasDev.Tabular/Analysis/CultureCatalog.cs b/src/TriasDev.Tabular/Analysis/CultureCatalog.cs index bef047e..3cbd5cb 100644 --- a/src/TriasDev.Tabular/Analysis/CultureCatalog.cs +++ b/src/TriasDev.Tabular/Analysis/CultureCatalog.cs @@ -39,6 +39,10 @@ public static bool TryGet(string? name, out CultureInfo culture) } } + /// The decimal separator a culture reads numbers with, or null for one that is not available. + public static string? DecimalSeparatorOf(string? name) => + TryGet(name, out CultureInfo culture) ? culture.NumberFormat.NumberDecimalSeparator : null; + /// The names this runtime has, in order; the invariant culture when it has none of them. public static IReadOnlyList Available(IReadOnlyList names) { diff --git a/src/TriasDev.Tabular/Analysis/HypothesisBuilder.cs b/src/TriasDev.Tabular/Analysis/HypothesisBuilder.cs index 223fe8e..b71f5b2 100644 --- a/src/TriasDev.Tabular/Analysis/HypothesisBuilder.cs +++ b/src/TriasDev.Tabular/Analysis/HypothesisBuilder.cs @@ -21,7 +21,7 @@ internal static class HypothesisBuilder /// /// The share of values a reading must account for before it is offered. Text is exempt. /// - public static IReadOnlyList Build(ColumnFacts facts, double minimumConfidence = 0) + public static IReadOnlyList Build(ColumnFacts facts, double minimumConfidence = 0, string? preferredDecimalSeparator = null) { ArgumentNullException.ThrowIfNull(facts); @@ -59,7 +59,8 @@ public static IReadOnlyList 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( /// /// How much a reading claims. Used only to order equally confident ones, narrowest first. /// - private static int Specificity(ColumnType type) => + /// + /// 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. + /// + 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; + + /// + /// 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. + /// + 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, }; } diff --git a/src/TriasDev.Tabular/Analysis/TabularAnalyzer.cs b/src/TriasDev.Tabular/Analysis/TabularAnalyzer.cs index cded2b9..a7867ac 100644 --- a/src/TriasDev.Tabular/Analysis/TabularAnalyzer.cs +++ b/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 profilers) + private SheetProfile BuildProfile(SheetInfo sheet, int rowCount, List profilers, char? delimiter) { List 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 + /// The decimal separator the sheet's evidence favours, for columns whose values read completely + /// under both — 48.137, a decimal or a grouped integer (#61) — or null when nothing does. + /// + /// + /// 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. + /// + private static string? PreferredDecimalSeparator(List profilers, char? delimiter) + { + int votes = profilers.Sum(p => Math.Sign(p.DecimalCommaEvidence())); + + return votes switch + { + > 0 => ",", + < 0 => ".", + _ => delimiter switch + { + ';' => ",", + ',' => ".", + _ => null, + }, + }; + } + /// The row up to and including its last value; empty when it holds none. private static ReadOnlySpan ToLastValue(ReadOnlySpan row) { diff --git a/src/TriasDev.Tabular/PublicAPI.Unshipped.txt b/src/TriasDev.Tabular/PublicAPI.Unshipped.txt index d74ed17..ec7da17 100644 --- a/src/TriasDev.Tabular/PublicAPI.Unshipped.txt +++ b/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 diff --git a/tests/TriasDev.Tabular.Tests/Analysis/AmbiguousNumberTests.cs b/tests/TriasDev.Tabular.Tests/Analysis/AmbiguousNumberTests.cs new file mode 100644 index 0000000..5c12767 --- /dev/null +++ b/tests/TriasDev.Tabular.Tests/Analysis/AmbiguousNumberTests.cs @@ -0,0 +1,104 @@ +using System.Text; + +using TriasDev.Tabular.Csv; +using TriasDev.Tabular.Tests.Fixtures; +using TriasDev.Tabular.Xlsx; + +using Xunit; + +namespace TriasDev.Tabular.Tests.Analysis; + +/// +/// A column of three-digit decimals — 48.137 — reads completely both as decimals and as German +/// grouped integers. The reading the evidence favours goes first, the extremes are measured under it, +/// and the facts say the readings disagree (#61). +/// +/// +/// The evidence, in order: the sheet's other numeric columns, whose separator is not in doubt; then a +/// csv's delimiter, as a comma-delimited file cannot use an unquoted comma for decimals and a +/// semicolon-delimited one is German as a rule; and, with nothing to go on, the decimal over the +/// grouped integer, one group being weak evidence of grouping. +/// +public sealed class AmbiguousNumberTests +{ + private static CancellationToken Token => TestContext.Current.CancellationToken; + + private static readonly AnalysisOptions Cultures = new() { Cultures = ["", "de-DE", "en-US"] }; + + private static SheetProfile Csv(string text) + { + using CsvCursor cursor = new(new MemoryStream(Encoding.UTF8.GetBytes(text)), "t.csv"); + return TabularAnalyzer.Analyze(cursor, Cultures, cancellationToken: Token).Sheets[0]; + } + + private static string CultureOf(TypeHypothesis hypothesis) => hypothesis.Culture ?? "(none)"; + + [Fact] + public void ReadsThreeDigitDecimalsInACommaDelimitedFileAsDecimals() + { + ColumnProfile lat = Csv("id,lat,lon\n1,48.137,11.575\n2,52.520,13.405\n").Columns[1]; + + Assert.Equal(ColumnType.Decimal, lat.Hypotheses[0].Type); + Assert.NotEqual("de-DE", CultureOf(lat.Hypotheses[0])); + Assert.Equal(48.137m, lat.Facts.MinNumeric); + Assert.Equal(52.52m, lat.Facts.MaxNumeric); + Assert.True(lat.Facts.NumberReadingsDisagree); + } + + [Fact] + public void ReadsThemAsGermanInASemicolonDelimitedFile() + { + ColumnProfile amount = Csv("id;amount\n1;1.250\n2;3.500\n").Columns[1]; + + Assert.Equal("de-DE", CultureOf(amount.Hypotheses[0])); + Assert.Equal(1250m, amount.Facts.MinNumeric); + Assert.Equal(3500m, amount.Facts.MaxNumeric); + Assert.True(amount.Facts.NumberReadingsDisagree); + } + + [Fact] + public void FollowsTheSeparatorTheSheetsOtherColumnsLeaveNoDoubtAbout() + { + // Tab-delimited, so the delimiter says nothing; the unambiguous column says comma. + ColumnProfile amount = Csv("price\tamount\n1,5\t1.250\n2,75\t3.500\n").Columns[1]; + + Assert.Equal("de-DE", CultureOf(amount.Hypotheses[0])); + Assert.Equal(1250m, amount.Facts.MinNumeric); + + ColumnProfile other = Csv("price\tamount\n1.5\t1.250\n2.75\t3.500\n").Columns[1]; + + Assert.NotEqual("de-DE", CultureOf(other.Hypotheses[0])); + Assert.Equal(1.25m, other.Facts.MinNumeric); + } + + [Fact] + public void PrefersTheDecimalWhenNothingElseDecides() + { + string rows = """lat48.13752.520"""; + using XlsxCursor cursor = new(new MemoryStream(new XlsxPackage().WithSheet("S", rows).Build()), cancellationToken: Token); + + ColumnProfile lat = TabularAnalyzer.Analyze(cursor, Cultures, cancellationToken: Token).Sheets[0].Columns[0]; + + Assert.Equal(ColumnType.Decimal, lat.Hypotheses[0].Type); + Assert.NotEqual("de-DE", CultureOf(lat.Hypotheses[0])); + Assert.Equal(48.137m, lat.Facts.MinNumeric); + } + + [Fact] + public void ChangesNothingWhereTheReadingsAgree() + { + SheetProfile sheet = Csv("id;n\n1;42\n2;7\n"); + + Assert.False(sheet.Columns[1].Facts.NumberReadingsDisagree); + Assert.Equal(ColumnType.Integer, sheet.Columns[1].Hypotheses[0].Type); + Assert.Equal("", CultureOf(sheet.Columns[1].Hypotheses[0])); + } + + [Fact] + public void SaysNothingWhenOnlyOneReadingTakesEveryValue() + { + ColumnProfile column = Csv("id;n\n1;1,5\n2;2,25\n").Columns[1]; + + Assert.False(column.Facts.NumberReadingsDisagree); + } +}