CTABLES: Always show computed categories even if they'd be zeros.
authorBen Pfaff <blp@cs.stanford.edu>
Sat, 18 Feb 2023 00:12:21 +0000 (16:12 -0800)
committerBen Pfaff <blp@cs.stanford.edu>
Sat, 18 Feb 2023 00:12:21 +0000 (16:12 -0800)
With CATEGORIES EXCLUDE=YES, empty computed categories weren't shown.  This
seems counterintuitive, so this commit fixes it.

Thanks to Frans Houweling for reporting this bug.

src/language/commands/ctables.c
tests/language/commands/ctables.at

index e53a43023db32966031de4721df8f7f321452b72..ffc284945eb5417921c6fd506f1157025865102f 100644 (file)
@@ -5445,17 +5445,24 @@ static void
 ctables_section_recurse_add_empty_categories (
   struct ctables_section *s,
   const struct ctables_category **cats[PIVOT_N_AXES], struct ccase *c,
-  enum pivot_axis_type a, size_t a_idx)
+  enum pivot_axis_type a, size_t a_idx, bool add)
 {
   if (a >= PIVOT_N_AXES)
-    ctables_cell_insert__ (s, c, cats);
+    {
+      if (add)
+        ctables_cell_insert__ (s, c, cats);
+    }
   else if (!s->nests[a] || a_idx >= s->nests[a]->n)
-    ctables_section_recurse_add_empty_categories (s, cats, c, a + 1, 0);
+    ctables_section_recurse_add_empty_categories (s, cats, c, a + 1, 0, add);
   else
     {
       const struct variable *var = s->nests[a]->vars[a_idx];
-      const struct ctables_categories *categories = s->table->categories[
-        var_get_dict_index (var)];
+      size_t idx = var_get_dict_index (var);
+      bool show_empty = s->table->show_empty[idx];
+      if (show_empty)
+        add = true;
+
+      const struct ctables_categories *categories = s->table->categories[idx];
       int width = var_get_width (var);
       const struct hmap *occurrences = &s->occurrences[a][a_idx];
       const struct ctables_occurrence *o;
@@ -5466,16 +5473,19 @@ ctables_section_recurse_add_empty_categories (
           value_clone (value, &o->value, width);
           cats[a][a_idx] = ctables_categories_match (categories, value, var);
           assert (cats[a][a_idx] != NULL);
-          ctables_section_recurse_add_empty_categories (s, cats, c, a, a_idx + 1);
+          ctables_section_recurse_add_empty_categories (s, cats, c,
+                                                        a, a_idx + 1, add);
         }
 
       for (size_t i = 0; i < categories->n_cats; i++)
         {
           const struct ctables_category *cat = &categories->cats[i];
-          if (cat->type == CCT_POSTCOMPUTE)
+          if (cat->type == CCT_POSTCOMPUTE
+              || (show_empty && cat->type == CCT_SUBTOTAL))
             {
               cats[a][a_idx] = cat;
-              ctables_section_recurse_add_empty_categories (s, cats, c, a, a_idx + 1);
+              ctables_section_recurse_add_empty_categories (s, cats, c,
+                                                            a, a_idx + 1, true);
             }
         }
     }
@@ -5484,7 +5494,6 @@ ctables_section_recurse_add_empty_categories (
 static void
 ctables_section_add_empty_categories (struct ctables_section *s)
 {
-  bool show_empty = false;
   for (size_t a = 0; a < PIVOT_N_AXES; a++)
     if (s->nests[a])
       for (size_t k = 0; k < s->nests[a]->n; k++)
@@ -5494,13 +5503,8 @@ ctables_section_add_empty_categories (struct ctables_section *s)
             size_t idx = var_get_dict_index (var);
             const struct ctables_categories *cats = s->table->categories[idx];
             if (s->table->show_empty[idx])
-              {
-                show_empty = true;
-                ctables_add_category_occurrences (var, &s->occurrences[a][k], cats);
-              }
+              ctables_add_category_occurrences (var, &s->occurrences[a][k], cats);
           }
-  if (!show_empty)
-    return;
 
   const struct ctables_category *layer_cats[s->nests[PIVOT_AXIS_LAYER]->n];
   const struct ctables_category *row_cats[s->nests[PIVOT_AXIS_ROW]->n];
@@ -5512,7 +5516,7 @@ ctables_section_add_empty_categories (struct ctables_section *s)
       [PIVOT_AXIS_COLUMN] = column_cats,
     };
   struct ccase *c = case_create (dict_get_proto (s->table->ctables->dict));
-  ctables_section_recurse_add_empty_categories (s, cats, c, 0, 0);
+  ctables_section_recurse_add_empty_categories (s, cats, c, 0, 0, false);
   case_unref (c);
 }
 
index 7f15631d5b014f4a418f971031430a79f08b2adb..108e455e90fd683ba6592441de1f01a209bf3701 100644 (file)
@@ -1236,7 +1236,7 @@ AT_CLEANUP
 
 AT_SETUP([CTABLES categories and EMPTY])
 AT_CHECK([ln $top_srcdir/tests/language/commands/nhtsa.sav . || cp $top_srcdir/tests/language/commands/nhtsa.sav .])
-AT_DATA([ctables.sps],
+AT_DATA([ctables.sps], [dnl
 DATA LIST LIST NOTABLE /class datum size.
 BEGIN DATA
 1 1 1
@@ -1295,6 +1295,96 @@ AT_CHECK([pspp ctables.sps -O box=unicode -O width=80], [0], [dnl
 ])
 AT_CLEANUP
 
+dnl PCOMPUTE needs to be an internal exception to omitting empty
+dnl categories, but the code didn't always implement that
+dnl properly.  This test guards against regression.
+AT_SETUP([CTABLES interaction between EMPTY and PCOMPUTE])
+AT_CHECK([ln $top_srcdir/tests/language/commands/nhtsa.sav . || cp $top_srcdir/tests/language/commands/nhtsa.sav .])
+AT_DATA([ctables.sps],
+[[GET 'nhtsa.sav'.
+
+* Make sure that "Never" has no responses.
+SELECT IF qn1 <> 5.
+
+* This will include "% Not Drivers" even though it has a zero value,
+because it is explicitly specified.  It will include "missing" and the
+"Not Drivers or Missing" subtotal, even though they are both zero,
+because we include empty values.
+CTABLES
+  /PCOMPUTE &all_drivers=EXPR([1 THRU 2] + [3 THRU 4])
+  /PPROPERTIES &all_drivers LABEL='All Drivers'
+  /PCOMPUTE &pct_never=EXPR([5] / ([1 THRU 2] + [3 THRU 4] + [5]) * 100)
+  /PPROPERTIES &pct_never LABEL='% Not Drivers' FORMAT=COUNT PCT40.1
+  /TABLE=qn1 BY qns3a
+  /CATEGORIES VARIABLES=qn1
+  [1 THRU 2, SUBTOTAL='Frequent Drivers',
+  3 THRU 4, SUBTOTAL='Infrequent Drivers',
+  &all_drivers, 5, &pct_never,
+  MISSING, SUBTOTAL='Not Drivers or Missing'].
+
+* This will include "% Not Drivers" even though it has a zero value,
+because it is explicitly specified.  It will omit "missing" and the
+"Not Drivers or Missing" subtotal because they are both zero and
+we have EMPTY=EXCLUDE.
+CTABLES
+  /PCOMPUTE &all_drivers=EXPR([1 THRU 2] + [3 THRU 4])
+  /PPROPERTIES &all_drivers LABEL='All Drivers'
+  /PCOMPUTE &pct_never=EXPR([5] / ([1 THRU 2] + [3 THRU 4] + [5]) * 100)
+  /PPROPERTIES &pct_never LABEL='% Not Drivers' FORMAT=COUNT PCT40.1
+  /TABLE=qn1 BY qns3a
+  /CATEGORIES VARIABLES=ALL EMPTY=EXCLUDE
+  /CATEGORIES VARIABLES=qn1
+  [1 THRU 2, SUBTOTAL='Frequent Drivers',
+  3 THRU 4, SUBTOTAL='Infrequent Drivers',
+  &all_drivers, 5, &pct_never,
+  MISSING, SUBTOTAL='Not Drivers or Missing'].
+]])
+AT_CHECK([pspp ctables.sps -O box=unicode], [0], [dnl
+                                 Custom Tables
+╭────────────────────────────────────────────────────────────────┬────────────╮
+│                                                                │S3a. GENDER:│
+│                                                                ├─────┬──────┤
+│                                                                │ Male│Female│
+│                                                                ├─────┼──────┤
+│                                                                │Count│ Count│
+├────────────────────────────────────────────────────────────────┼─────┼──────┤
+│ 1. How often do you usually drive a car or Every day           │ 2305│  2362│
+│other motor vehicle?                        Several days a week │  440│   834│
+│                                            Frequent Drivers    │ 2745│  3196│
+│                                            Once a week or less │  125│   236│
+│                                            Only certain times a│   58│    72│
+│                                            year                │     │      │
+│                                            Infrequent Drivers  │  183│   308│
+│                                            All Drivers         │ 2928│  3504│
+│                                            Never               │    0│     0│
+│                                            % Not Drivers       │  .0%│   .0%│
+│                                            Don't know          │    0│     0│
+│                                            Refused             │    0│     0│
+│                                            Not Drivers or      │    0│     0│
+│                                            Missing             │     │      │
+╰────────────────────────────────────────────────────────────────┴─────┴──────╯
+
+                                 Custom Tables
+╭────────────────────────────────────────────────────────────────┬────────────╮
+│                                                                │S3a. GENDER:│
+│                                                                ├─────┬──────┤
+│                                                                │ Male│Female│
+│                                                                ├─────┼──────┤
+│                                                                │Count│ Count│
+├────────────────────────────────────────────────────────────────┼─────┼──────┤
+│ 1. How often do you usually drive a car or Every day           │ 2305│  2362│
+│other motor vehicle?                        Several days a week │  440│   834│
+│                                            Frequent Drivers    │ 2745│  3196│
+│                                            Once a week or less │  125│   236│
+│                                            Only certain times a│   58│    72│
+│                                            year                │     │      │
+│                                            Infrequent Drivers  │  183│   308│
+│                                            All Drivers         │ 2928│  3504│
+│                                            % Not Drivers       │  .0%│   .0%│
+╰────────────────────────────────────────────────────────────────┴─────┴──────╯
+])
+AT_CLEANUP
+
 AT_SETUP([CTABLES sorting categories])
 AT_CHECK([ln $top_srcdir/tests/language/commands/nhtsa.sav . || cp $top_srcdir/tests/language/commands/nhtsa.sav .])
 AT_DATA([ctables.sps],
@@ -1924,12 +2014,12 @@ AT_CHECK([pspp ctables.sps -O box=unicode -O width=120], [0], [dnl
 │A. Get stopped by the police?                Very likely│       .│   24.9%│   18.5%│   24.0%│  26.6%│   25.5%│   33.3%│
 │                                             Somewhat   │       .│   38.3%│   41.9%│   38.6%│  37.5%│   36.4%│   23.8%│
 │                                             likely     │        │        │        │        │       │        │        │
-│                                             Subtotal   │        │   72.8%│   68.6%│   75.0%│  74.0%│   81.8%│   81.0%│
+│                                             Subtotal   │       .│   72.8%│   68.6%│   75.0%│  74.0%│   81.8%│   81.0%│
 │                                             Somewhat   │       .│   18.1%│   21.7%│   16.8%│  16.7%│   10.9%│    9.5%│
 │                                             unlikely   │        │        │        │        │       │        │        │
 │                                             Very       │       .│    9.2%│    9.7%│    8.2%│   9.4%│    7.3%│    9.5%│
 │                                             unlikely   │        │        │        │        │       │        │        │
-│                                             Subtotal   │        │   27.2%│   31.4%│   25.0%│  26.0%│   18.2%│   19.0%│
+│                                             Subtotal   │       .│   27.2%│   31.4%│   25.0%│  26.0%│   18.2%│   19.0%│
 ╰────────────────────────────────────────────────────────┴────────┴────────┴────────┴────────┴───────┴────────┴────────╯
 
                                                       Custom Tables
@@ -2204,7 +2294,7 @@ AT_CHECK([pspp ctables.sps -O box=unicode -O width=120], [0], [dnl
 │Favorite month JAN│ 10│  3│  8│  6│  4│  2│  7│     17│   40│  9│  6│  4│  2│  7│  5│  3│     12│   36│
 │               FEB│  6│  8│  6│  4│  2│  7│  5│     11│   38│ 12│  4│  2│  7│  5│  3│  8│     20│   41│
 │               MAR│ 16│  6│  4│  2│  7│  5│  3│     19│   43│  8│  2│  7│  5│  3│  8│  6│     14│   39│
-│               Q1 │ 32│ 17│ 18│ 12│ 13│ 14│ 15│       │     │ 29│ 12│ 13│ 14│ 15│ 16│ 17│       │     │
+│               Q1 │ 32│ 17│ 18│ 12│ 13│ 14│ 15│     47│     │ 29│ 12│ 13│ 14│ 15│ 16│ 17│     46│     │
 │               APR│ 12│  4│  2│  7│  5│  3│  8│     20│   41│  4│  7│  5│  3│  8│  6│  4│      8│   37│
 │               MAY│  8│  2│  7│  5│  3│  8│  6│     14│   39│ 14│  5│  3│  8│  6│  4│  2│     16│   42│
 │               JUN│  4│  7│  5│  3│  8│  6│  4│      8│   37│ 10│  3│  8│  6│  4│  2│  7│     17│   40│
@@ -2212,11 +2302,11 @@ AT_CHECK([pspp ctables.sps -O box=unicode -O width=120], [0], [dnl
 │               JUL│ 14│  5│  3│  8│  6│  4│  2│     16│   42│  6│  8│  6│  4│  2│  7│  5│     11│   38│
 │               AUG│ 10│  3│  8│  6│  4│  2│  7│     17│   40│ 16│  6│  4│  2│  7│  5│  3│     19│   43│
 │               SEP│  6│  8│  6│  4│  2│  7│  5│     11│   38│ 12│  4│  2│  7│  5│  3│  8│     20│   41│
-│               Q3 │ 54│ 29│ 31│ 33│ 28│ 30│ 32│       │     │ 62│ 33│ 28│ 30│ 32│ 27│ 29│       │     │
+│               Q3 │ 54│ 29│ 31│ 33│ 28│ 30│ 32│     86│     │ 62│ 33│ 28│ 30│ 32│ 27│ 29│     91│     │
 │               OCT│ 16│  6│  4│  2│  7│  5│  3│     19│   43│  8│  2│  7│  5│  3│  8│  6│     14│   39│
 │               NOV│ 12│  4│  2│  7│  5│  3│  8│     20│   41│  4│  7│  5│  3│  8│  6│  4│      8│   37│
 │               DEC│  8│  2│  7│  5│  3│  8│  6│     14│   39│ 14│  5│  3│  8│  6│  4│  2│     16│   42│
-│               Q4 │ 36│ 12│ 13│ 14│ 15│ 16│ 17│       │     │ 26│ 14│ 15│ 16│ 17│ 18│ 12│       │     │
+│               Q4 │ 36│ 12│ 13│ 14│ 15│ 16│ 17│     53│     │ 26│ 14│ 15│ 16│ 17│ 18│ 12│     38│     │
 ╰──────────────────┴───┴───┴───┴───┴───┴───┴───┴───────┴─────┴───┴───┴───┴───┴───┴───┴───┴───────┴─────╯
 ])
 AT_CLEANUP