Merge PR #3219

crates/engine/src/cmd/pivot/fields.rs

@@ -507,9 +507,13 @@ pub(super) fn value_settings(s: &mut Session, p: &Json) -> Result<Json> {
         let mut v = cur.clone();
         if let Some(f) = func {
             v.func = f;
-            // An automatic caption follows the function.
-            let auto = auto_name(cur.func, &cur.source_col);
-            if cur.name.eq_ignore_ascii_case(&auto) || cur.name.strip_prefix(auto.as_str()).is_some_and(|r| r.chars().all(|c| c.is_ascii_digit())) {
+            // An automatic caption follows the function. Count Numbers was captioned "Count
+            // Numbers of …" before it took Excel's "Count of …", and saved workbooks still carry it.
+            let is_auto = |base: &str| {
+                cur.name.eq_ignore_ascii_case(base) || cur.name.strip_prefix(base).is_some_and(|r| r.chars().all(|c| c.is_ascii_digit()))
+            };
+            let legacy = cur.func == PivotFunc::CountNumbers && is_auto(&format!("Count Numbers of {}", cur.source_col));
+            if is_auto(&auto_name(cur.func, &cur.source_col)) || legacy {
                 v.name = unique_value_name(pt, &auto_name(f, &cur.source_col), Some(i));
             }
         }

crates/engine/src/cmd/pivot/tests.rs

@@ -1013,3 +1013,36 @@ fn detail_cells_are_the_cells_show_details_takes() {
     let pt = &d.wb.active().unwrap().pivots[0];
     assert!(crate::pivot::detail_cells(&d.wb, pt, d.wb.active_sheet()).unwrap_err().contains("out of date"));
 }
+
+#[test]
+fn rank_ties_are_dense_and_count_numbers_is_captioned_count_of() {
+    // #1448 items 10 and 12: Excel ranks 5, 4, 3, 3, 4 as 1, 2, 3, 3, 2 (GridCraft gave 1, 2, 4,
+    // 4, 2), and captions a Count Numbers field "Count of Val".
+    let mut s = Session::new();
+    s.new_workbook();
+    let rows = json!([["Key", "Val"], ["A", 5], ["B", 4], ["C", 3], ["D", 3], ["E", 4]]);
+    s.execute("range.setValues", json!({"range": "A1", "values": rows})).unwrap();
+    s.execute("insert.pivotTable", json!({"source": "Sheet1!$A$1:$B$6", "destination": "Sheet1!D3", "rows": ["Key"], "values": ["Val"]})).unwrap();
+    s.execute("pivot.valueSettings", json!({"index": 0, "showAs": "rank"})).unwrap();
+    let ranks: Vec<Value> = ["E4", "E5", "E6", "E7", "E8"].iter().map(|a| val(&s, a)).collect();
+    assert_eq!(ranks, [1.0, 2.0, 3.0, 3.0, 2.0].map(Value::Number));
+    s.execute("pivot.valueSettings", json!({"index": 0, "showAs": "normal", "func": "countNumbers"})).unwrap();
+    assert_eq!(text(&s, "E3"), "Count of Val");
+}
+
+#[test]
+fn legacy_count_numbers_caption_follows_the_function() {
+    // Workbooks saved before Count Numbers was captioned "Count of" carry "Count Numbers of Val";
+    // that is still an automatic caption, so it follows a new function. A custom name stays.
+    let mut s = Session::new();
+    s.new_workbook();
+    let rows = json!([["Key", "Val"], ["A", 5], ["B", 4]]);
+    s.execute("range.setValues", json!({"range": "A1", "values": rows})).unwrap();
+    s.execute("insert.pivotTable", json!({"source": "Sheet1!$A$1:$B$3", "destination": "Sheet1!D3", "rows": ["Key"], "values": ["Val"]})).unwrap();
+    for (legacy, after) in [("Count Numbers of Val", "Sum of Val"), ("Count Numbers of Val2", "Sum of Val"), ("Numbers counted", "Numbers counted")] {
+        s.execute("pivot.valueSettings", json!({"index": 0, "func": "countNumbers", "name": legacy})).unwrap();
+        assert_eq!(text(&s, "E3"), legacy);
+        s.execute("pivot.valueSettings", json!({"index": 0, "func": "sum"})).unwrap();
+        assert_eq!(text(&s, "E3"), after, "{legacy}");
+    }
+}

crates/engine/src/pivot/compute.rs

@@ -355,15 +355,19 @@ fn ranked_value(raw: Value, lookup: &ValueLookup<'_>, context: &ShowContext<'_>)
     let Some((_, parent)) = context.row_path.split_last() else { return Value::Empty };
     let Some(me) = raw.as_f64() else { return raw };
     let siblings = context.siblings.get(parent).cloned().unwrap_or_default();
-    let mut rank = 1u64;
-    for sibling in siblings {
-        let mut path = parent.to_vec();
-        path.push(sibling);
-        if number(&lookup.get(&path, context.col_path, context.value_index)).is_some_and(|value| value > me) {
-            rank += 1;
-        }
-    }
-    Value::number(rank as f64)
+    // Excel's ranks are dense: ties share a rank and the next value takes the next one
+    // (5, 4, 3, 3, 4 rank 1, 2, 3, 3, 2), so count the distinct larger values.
+    let mut larger: Vec<f64> = siblings
+        .into_iter()
+        .filter_map(|sibling| {
+            let mut path = parent.to_vec();
+            path.push(sibling);
+            number(&lookup.get(&path, context.col_path, context.value_index)).filter(|value| *value > me)
+        })
+        .collect();
+    larger.sort_by(f64::total_cmp);
+    larger.dedup();
+    Value::number(larger.len() as f64 + 1.0)
 }
 
 fn show_value(definition: &PivotValue, raw: Value, lookup: &ValueLookup<'_>, context: &ShowContext<'_>) -> Value {

crates/engine/src/pivot/items.rs

@@ -197,7 +197,8 @@ impl Acc {
     }
 }
 
-/// Excel's caption verb for a function ("Sum of Sales").
+/// Excel's caption verb for a function ("Sum of Sales"). Count Numbers is captioned "Count of
+/// Sales", as Excel names it and as the XLSX reader names a value field saved without a name.
 pub fn func_caption(f: PivotFunc) -> &'static str {
     match f {
         PivotFunc::Sum => "Sum",
@@ -206,7 +207,7 @@ pub fn func_caption(f: PivotFunc) -> &'static str {
         PivotFunc::Max => "Max",
         PivotFunc::Min => "Min",
         PivotFunc::Product => "Product",
-        PivotFunc::CountNumbers => "Count Numbers",
+        PivotFunc::CountNumbers => "Count",
         PivotFunc::StdDev => "StdDev",
         PivotFunc::StdDevP => "StdDevp",
         PivotFunc::Var => "Var",