improve error messages for scale variable summaries
[pspp] / src / language / stats / ctables.c
index e54be6d64bcfcb9d602438285945e8a41dad455e..3e370b8276953a5ebc6445cd7601f06bee6e824f 100644 (file)
@@ -854,6 +854,7 @@ ctables_summary_spec_set_uninit (struct ctables_summary_spec_set *set)
 {
   for (size_t i = 0; i < set->n; i++)
     ctables_summary_spec_uninit (&set->specs[i]);
+  free (set->listwise_vars);
   free (set->specs);
 }
 
@@ -904,102 +905,9 @@ ctables_function_availability (enum ctables_summary_function f)
 static bool
 ctables_summary_function_is_count (enum ctables_summary_function f)
 {
-  switch (f)
-    {
-    case CTSF_COUNT:
-    case CTSF_ECOUNT:
-    case CTSF_ROWPCT_COUNT:
-    case CTSF_COLPCT_COUNT:
-    case CTSF_TABLEPCT_COUNT:
-    case CTSF_SUBTABLEPCT_COUNT:
-    case CTSF_LAYERPCT_COUNT:
-    case CTSF_LAYERROWPCT_COUNT:
-    case CTSF_LAYERCOLPCT_COUNT:
-    case CTSF_UCOUNT:
-    case CTSF_UROWPCT_COUNT:
-    case CTSF_UCOLPCT_COUNT:
-    case CTSF_UTABLEPCT_COUNT:
-    case CTSF_USUBTABLEPCT_COUNT:
-    case CTSF_ULAYERPCT_COUNT:
-    case CTSF_ULAYERROWPCT_COUNT:
-    case CTSF_ULAYERCOLPCT_COUNT:
-      return true;
-
-    case CTSF_ROWPCT_VALIDN:
-    case CTSF_COLPCT_VALIDN:
-    case CTSF_TABLEPCT_VALIDN:
-    case CTSF_SUBTABLEPCT_VALIDN:
-    case CTSF_LAYERPCT_VALIDN:
-    case CTSF_LAYERROWPCT_VALIDN:
-    case CTSF_LAYERCOLPCT_VALIDN:
-    case CTSF_ROWPCT_TOTALN:
-    case CTSF_COLPCT_TOTALN:
-    case CTSF_TABLEPCT_TOTALN:
-    case CTSF_SUBTABLEPCT_TOTALN:
-    case CTSF_LAYERPCT_TOTALN:
-    case CTSF_LAYERROWPCT_TOTALN:
-    case CTSF_LAYERCOLPCT_TOTALN:
-    case CTSF_MAXIMUM:
-    case CTSF_MEAN:
-    case CTSF_MEDIAN:
-    case CTSF_MINIMUM:
-    case CTSF_MISSING:
-    case CTSF_MODE:
-    case CTSF_PTILE:
-    case CTSF_RANGE:
-    case CTSF_SEMEAN:
-    case CTSF_STDDEV:
-    case CTSF_SUM:
-    case CSTF_TOTALN:
-    case CTSF_ETOTALN:
-    case CTSF_VALIDN:
-    case CTSF_EVALIDN:
-    case CTSF_VARIANCE:
-    case CTSF_ROWPCT_SUM:
-    case CTSF_COLPCT_SUM:
-    case CTSF_TABLEPCT_SUM:
-    case CTSF_SUBTABLEPCT_SUM:
-    case CTSF_LAYERPCT_SUM:
-    case CTSF_LAYERROWPCT_SUM:
-    case CTSF_LAYERCOLPCT_SUM:
-    case CTSF_UROWPCT_VALIDN:
-    case CTSF_UCOLPCT_VALIDN:
-    case CTSF_UTABLEPCT_VALIDN:
-    case CTSF_USUBTABLEPCT_VALIDN:
-    case CTSF_ULAYERPCT_VALIDN:
-    case CTSF_ULAYERROWPCT_VALIDN:
-    case CTSF_ULAYERCOLPCT_VALIDN:
-    case CTSF_UROWPCT_TOTALN:
-    case CTSF_UCOLPCT_TOTALN:
-    case CTSF_UTABLEPCT_TOTALN:
-    case CTSF_USUBTABLEPCT_TOTALN:
-    case CTSF_ULAYERPCT_TOTALN:
-    case CTSF_ULAYERROWPCT_TOTALN:
-    case CTSF_ULAYERCOLPCT_TOTALN:
-    case CTSF_UMEAN:
-    case CTSF_UMEDIAN:
-    case CTSF_UMISSING:
-    case CTSF_UMODE:
-    case CTSF_UPTILE:
-    case CTSF_USEMEAN:
-    case CTSF_USTDDEV:
-    case CTSF_USUM:
-    case CSTF_UTOTALN:
-    case CTSF_UVALIDN:
-    case CTSF_UVARIANCE:
-    case CTSF_UROWPCT_SUM:
-    case CTSF_UCOLPCT_SUM:
-    case CTSF_UTABLEPCT_SUM:
-    case CTSF_USUBTABLEPCT_SUM:
-    case CTSF_ULAYERPCT_SUM:
-    case CTSF_ULAYERROWPCT_SUM:
-    case CTSF_ULAYERCOLPCT_SUM:
-      return false;
-  }
-  NOT_REACHED ();
+  return f == CTSF_COUNT || f == CTSF_ECOUNT || f == CTSF_UCOUNT;
 }
 
-
 static bool
 parse_ctables_summary_function (struct lexer *lexer,
                                 enum ctables_summary_function *f)
@@ -1730,7 +1638,10 @@ ctables_table_parse_explicit_category (struct lexer *lexer,
           else
             {
               if (!lex_force_string (lexer))
-                return false;
+                {
+                  ss_dealloc (&s);
+                  return false;
+                }
               struct substring sr1 = parse_substring (lexer, dict);
               *cat = cct_srange (s, sr1);
             }
@@ -2374,6 +2285,8 @@ ctables_nest_uninit (struct ctables_nest *nest)
   free (nest->vars);
   for (enum ctables_summary_variant sv = 0; sv < N_CSVS; sv++)
     ctables_summary_spec_set_uninit (&nest->specs[sv]);
+  for (enum ctables_domain_type dt = 0; dt < N_CTDTS; dt++)
+    free (nest->domains[dt]);
 }
 
 static void
@@ -2404,7 +2317,6 @@ nest_fts (struct ctables_stack s0, struct ctables_stack s1)
 
         size_t allocate = a->n + b->n;
         struct variable **vars = xnmalloc (allocate, sizeof *vars);
-        enum pivot_axis_type *axes = xnmalloc (allocate, sizeof *axes);
         size_t n = 0;
         for (size_t k = 0; k < a->n; k++)
           vars[n++] = a->vars[k];
@@ -3442,10 +3354,19 @@ ctables_cell_compare_3way (const void *a_, const void *b_, const void *aux_)
             {
               const char *a_label = var_lookup_value_label (var, a_val);
               const char *b_label = var_lookup_value_label (var, b_val);
-              int cmp = (a_label
-                         ? (b_label ? strcmp (a_label, b_label) : 1)
-                         : (b_label ? -1 : value_compare_3way (
-                              a_val, b_val, var_get_width (var))));
+              int cmp;
+              if (a_label)
+                {
+                  if (!b_label)
+                    return -1;
+                  cmp = strcmp (a_label, b_label);
+                }
+              else
+                {
+                  if (b_label)
+                    return 1;
+                  cmp = value_compare_3way (a_val, b_val, var_get_width (var));
+                }
               if (cmp)
                 return a_cv->category->sort_ascending ? cmp : -cmp;
             }
@@ -4446,6 +4367,37 @@ ctables_cell_calculate_postcompute (const struct ctables_section *s,
   return ctables_pcexpr_evaluate (&ctx, pc->expr);
 }
 
+static char *
+ctables_format (double d, const struct fmt_spec *format,
+                const struct fmt_settings *settings)
+{
+  const union value v = { .f = d };
+  char *s = data_out_stretchy (&v, "UTF-8", format, settings, NULL);
+
+  /* The custom-currency specifications for NEQUAL, PAREN, and PCTPAREN don't
+     produce the results we want for negative numbers, putting the negative
+     sign in the wrong spot, before the prefix instead of after it.  We can't,
+     in fact, produce the desired results using a custom-currency
+     specification.  Instead, we postprocess the output, moving the negative
+     sign into place:
+
+         NEQUAL:   "-N=3"  => "N=-3"
+         PAREN:    "-(3)"  => "(-3)"
+         PCTPAREN: "-(3%)" => "(-3%)"
+
+     This transformation doesn't affect NEGPAREN. */
+  char *minus_src = strchr (s, '-');
+  if (minus_src && (minus_src == s || minus_src[-1] != 'E'))
+    {
+      char *n_equals = strstr (s, "N=");
+      char *lparen = strchr (s, '(');
+      char *minus_dst = n_equals ? n_equals + 1 : lparen;
+      if (minus_dst)
+        move_element (s, minus_dst - s + 1, 1, minus_src - s, minus_dst - s);
+    }
+  return s;
+}
+
 static void
 ctables_table_output (struct ctables *ct, struct ctables_table *t)
 {
@@ -4772,17 +4724,14 @@ ctables_table_output (struct ctables *ct, struct ctables_table *t)
               else if (d == SYSMIS && ct->missing)
                 value = pivot_value_new_user_text (ct->missing, SIZE_MAX);
               else if (is_ctables_format)
-                {
-                  char *s = data_out_stretchy (&(union value) { .f = d },
-                                               "UTF-8", &format,
-                                               &ct->ctables_formats, NULL);
-                  value = pivot_value_new_user_text_nocopy (s);
-                }
+                value = pivot_value_new_user_text_nocopy (
+                  ctables_format (d, &format, &ct->ctables_formats));
               else
                 {
                   value = pivot_value_new_number (d);
                   value->numeric.format = format;
                 }
+              /* XXX should text values be right-justified? */
               pivot_table_put (pt, dindexes, n_dindexes, value);
             }
         }
@@ -5043,10 +4992,13 @@ ctables_prepare_table (struct ctables_table *t)
                   listwise_vars[n++] = other_nest->vars[other_nest->scale_idx];
                 }
             }
-          for (size_t j = 0; j < N_CSVS; j++)
+          for (enum ctables_summary_variant sv = 0; sv < N_CSVS; sv++)
             {
-              nest->specs[j].listwise_vars = listwise_vars;
-              nest->specs[j].n_listwise_vars = n;
+              if (sv > 0)
+                listwise_vars = xmemdup (listwise_vars,
+                                         n * sizeof *listwise_vars);
+              nest->specs[sv].listwise_vars = listwise_vars;
+              nest->specs[sv].n_listwise_vars = n;
             }
         }
     }
@@ -5401,11 +5353,6 @@ ctables_section_uninit (struct ctables_section *s)
       for (size_t i = 0; i < nest->n; i++)
         hmap_destroy (&s->occurrences[a][i]);
       free (s->occurrences[a]);
-      for (enum ctables_domain_type dt = 0; dt < N_CTDTS; dt++)
-        {
-          free (nest->domains[dt]);
-          nest->domains[dt] = NULL;
-        }
     }
 
   hmap_destroy (&s->cells);
@@ -5579,7 +5526,7 @@ struct operator
   };
 
 static const struct operator *
-ctable_pcexpr_match_operator (struct lexer *lexer,
+ctables_pcexpr_match_operator (struct lexer *lexer,
                               const struct operator ops[], size_t n_ops)
 {
   for (const struct operator *op = ops; op < ops + n_ops; op++)
@@ -5595,7 +5542,7 @@ ctable_pcexpr_match_operator (struct lexer *lexer,
 }
 
 static struct ctables_pcexpr *
-ctable_pcexpr_parse_binary_operators__ (
+ctables_pcexpr_parse_binary_operators__ (
   struct lexer *lexer, struct dictionary *dict,
   const struct operator ops[], size_t n_ops,
   parse_recursively_func *parse_next_level,
@@ -5604,7 +5551,7 @@ ctable_pcexpr_parse_binary_operators__ (
   for (int op_count = 0; ; op_count++)
     {
       const struct operator *op
-        = ctable_pcexpr_match_operator (lexer, ops, n_ops);
+        = ctables_pcexpr_match_operator (lexer, ops, n_ops);
       if (!op)
         {
           if (op_count > 1 && chain_warning)
@@ -5625,23 +5572,22 @@ ctable_pcexpr_parse_binary_operators__ (
 }
 
 static struct ctables_pcexpr *
-ctable_pcexpr_parse_binary_operators (struct lexer *lexer,
-                                      struct dictionary *dict,
-                                      const struct operator ops[], size_t n_ops,
-                                      parse_recursively_func *parse_next_level,
-                                      const char *chain_warning)
+ctables_pcexpr_parse_binary_operators (
+  struct lexer *lexer, struct dictionary *dict,
+  const struct operator ops[], size_t n_ops,
+  parse_recursively_func *parse_next_level, const char *chain_warning)
 {
   struct ctables_pcexpr *lhs = parse_next_level (lexer, dict);
   if (!lhs)
     return NULL;
 
-  return ctable_pcexpr_parse_binary_operators__ (lexer, dict, ops, n_ops,
+  return ctables_pcexpr_parse_binary_operators__ (lexer, dict, ops, n_ops,
                                                  parse_next_level,
                                                  chain_warning, lhs);
 }
 
-static struct ctables_pcexpr *ctable_pcexpr_parse_add (struct lexer *,
-                                                       struct dictionary *);
+static struct ctables_pcexpr *ctables_pcexpr_parse_add (struct lexer *,
+                                                        struct dictionary *);
 
 static struct ctables_pcexpr
 ctpo_cat_nrange (double low, double high)
@@ -5662,7 +5608,7 @@ ctpo_cat_srange (struct substring low, struct substring high)
 }
 
 static struct ctables_pcexpr *
-ctable_pcexpr_parse_primary (struct lexer *lexer, struct dictionary *dict)
+ctables_pcexpr_parse_primary (struct lexer *lexer, struct dictionary *dict)
 {
   int start_ofs = lex_ofs (lexer);
   struct ctables_pcexpr e;
@@ -5779,7 +5725,7 @@ ctable_pcexpr_parse_primary (struct lexer *lexer, struct dictionary *dict)
     }
   else if (lex_match (lexer, T_LPAREN))
     {
-      struct ctables_pcexpr *ep = ctable_pcexpr_parse_add (lexer, dict);
+      struct ctables_pcexpr *ep = ctables_pcexpr_parse_add (lexer, dict);
       if (!ep)
         return NULL;
       if (!lex_force_match (lexer, T_RPAREN))
@@ -5813,7 +5759,7 @@ ctables_pcexpr_allocate_neg (struct ctables_pcexpr *sub,
 }
 
 static struct ctables_pcexpr *
-ctable_pcexpr_parse_exp (struct lexer *lexer, struct dictionary *dict)
+ctables_pcexpr_parse_exp (struct lexer *lexer, struct dictionary *dict)
 {
   static const struct operator op = { T_EXP, CTPO_POW };
 
@@ -5823,9 +5769,9 @@ ctable_pcexpr_parse_exp (struct lexer *lexer, struct dictionary *dict)
       "To disable this warning, insert parentheses.");
 
   if (lex_token (lexer) != T_NEG_NUM || lex_next_token (lexer, 1) != T_EXP)
-    return ctable_pcexpr_parse_binary_operators (lexer, dict, &op, 1,
-                                                 ctable_pcexpr_parse_primary,
-                                                 chain_warning);
+    return ctables_pcexpr_parse_binary_operators (lexer, dict, &op, 1,
+                                                  ctables_pcexpr_parse_primary,
+                                                  chain_warning);
 
   /* Special case for situations like "-5**6", which must be parsed as
      -(5**6). */
@@ -5839,9 +5785,9 @@ ctable_pcexpr_parse_exp (struct lexer *lexer, struct dictionary *dict)
   };
   lex_get (lexer);
 
-  struct ctables_pcexpr *node = ctable_pcexpr_parse_binary_operators__ (
+  struct ctables_pcexpr *node = ctables_pcexpr_parse_binary_operators__ (
     lexer, dict, &op, 1,
-    ctable_pcexpr_parse_primary, chain_warning, lhs);
+    ctables_pcexpr_parse_primary, chain_warning, lhs);
   if (!node)
     return NULL;
 
@@ -5850,13 +5796,13 @@ ctable_pcexpr_parse_exp (struct lexer *lexer, struct dictionary *dict)
 
 /* Parses the unary minus level. */
 static struct ctables_pcexpr *
-ctable_pcexpr_parse_neg (struct lexer *lexer, struct dictionary *dict)
+ctables_pcexpr_parse_neg (struct lexer *lexer, struct dictionary *dict)
 {
   int start_ofs = lex_ofs (lexer);
   if (!lex_match (lexer, T_DASH))
-    return ctable_pcexpr_parse_exp (lexer, dict);
+    return ctables_pcexpr_parse_exp (lexer, dict);
 
-  struct ctables_pcexpr *inner = ctable_pcexpr_parse_neg (lexer, dict);
+  struct ctables_pcexpr *inner = ctables_pcexpr_parse_neg (lexer, dict);
   if (!inner)
     return NULL;
 
@@ -5865,7 +5811,7 @@ ctable_pcexpr_parse_neg (struct lexer *lexer, struct dictionary *dict)
 
 /* Parses the multiplication and division level. */
 static struct ctables_pcexpr *
-ctable_pcexpr_parse_mul (struct lexer *lexer, struct dictionary *dict)
+ctables_pcexpr_parse_mul (struct lexer *lexer, struct dictionary *dict)
 {
   static const struct operator ops[] =
     {
@@ -5873,14 +5819,14 @@ ctable_pcexpr_parse_mul (struct lexer *lexer, struct dictionary *dict)
       { T_SLASH, CTPO_DIV },
     };
 
-  return ctable_pcexpr_parse_binary_operators (lexer, dict, ops,
+  return ctables_pcexpr_parse_binary_operators (lexer, dict, ops,
                                                sizeof ops / sizeof *ops,
-                                               ctable_pcexpr_parse_neg, NULL);
+                                               ctables_pcexpr_parse_neg, NULL);
 }
 
 /* Parses the addition and subtraction level. */
 static struct ctables_pcexpr *
-ctable_pcexpr_parse_add (struct lexer *lexer, struct dictionary *dict)
+ctables_pcexpr_parse_add (struct lexer *lexer, struct dictionary *dict)
 {
   static const struct operator ops[] =
     {
@@ -5889,9 +5835,9 @@ ctable_pcexpr_parse_add (struct lexer *lexer, struct dictionary *dict)
       { T_NEG_NUM, CTPO_ADD },
     };
 
-  return ctable_pcexpr_parse_binary_operators (lexer, dict,
+  return ctables_pcexpr_parse_binary_operators (lexer, dict,
                                                ops, sizeof ops / sizeof *ops,
-                                               ctable_pcexpr_parse_mul, NULL);
+                                               ctables_pcexpr_parse_mul, NULL);
 }
 
 static struct ctables_postcompute *
@@ -5931,10 +5877,11 @@ ctables_parse_pcompute (struct lexer *lexer, struct dictionary *dict,
     }
 
   int expr_start = lex_ofs (lexer);
-  struct ctables_pcexpr *expr = ctable_pcexpr_parse_add (lexer, dict);
+  struct ctables_pcexpr *expr = ctables_pcexpr_parse_add (lexer, dict);
   int expr_end = lex_ofs (lexer) - 1;
   if (!expr || !lex_force_match (lexer, T_RPAREN))
     {
+      ctables_pcexpr_destroy (expr);
       free (name);
       return false;
     }
@@ -6549,15 +6496,21 @@ cmd_ctables (struct lexer *lexer, struct dataset *ds)
       if (n_summaries > 1)
         {
           msg (SE, _("Summaries may appear only on one axis."));
-          if (summaries[PIVOT_AXIS_ROW])
-            msg_at (SN, summaries[PIVOT_AXIS_ROW]->loc,
-                    _("This variable on the rows axis has a summary."));
-          if (summaries[PIVOT_AXIS_COLUMN])
-            msg_at (SN, summaries[PIVOT_AXIS_COLUMN]->loc,
-                    _("This variable on the columns axis has a summary."));
-          if (summaries[PIVOT_AXIS_LAYER])
-            msg_at (SN, summaries[PIVOT_AXIS_LAYER]->loc,
-                    _("This variable on the layers axis has a summary."));
+          for (enum pivot_axis_type a = 0; a < PIVOT_N_AXES; a++)
+            if (summaries[a])
+              {
+                msg_at (SN, summaries[a]->loc,
+                        a == PIVOT_AXIS_ROW
+                        ? _("This variable on the rows axis has a summary.")
+                        : a == PIVOT_AXIS_COLUMN
+                        ? _("This variable on the columns axis has a summary.")
+                        : _("This variable on the layers axis has a summary."));
+                if (scales[a])
+                  msg_at (SN, summaries[a]->loc,
+                          _("This is a scale variable, so it always has a "
+                            "summary even if the syntax does not explicitly "
+                            "specify one."));
+              }
           goto error;
         }
       for (enum pivot_axis_type a = 0; a < PIVOT_N_AXES; a++)