From a0f5f851c81999933a81fd9f9b455c92cce9a2c3 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Fri, 17 Feb 2023 16:12:21 -0800 Subject: [PATCH] CTABLES: Always show computed categories even if they'd be zeros. 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 | 36 +++++----- tests/language/commands/ctables.at | 102 +++++++++++++++++++++++++++-- 2 files changed, 116 insertions(+), 22 deletions(-) diff --git a/src/language/commands/ctables.c b/src/language/commands/ctables.c index e53a43023d..ffc284945e 100644 --- a/src/language/commands/ctables.c +++ b/src/language/commands/ctables.c @@ -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); } diff --git a/tests/language/commands/ctables.at b/tests/language/commands/ctables.at index 7f15631d5b..108e455e90 100644 --- a/tests/language/commands/ctables.at +++ b/tests/language/commands/ctables.at @@ -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 -- 2.30.2