Re: [PATCH] boxplot: introduce a default factor

[sfeam <sf...@us...>]

On Saturday, 03 January 2015 12:35:27 PM Jouke Witteveen wrote:
> When a row in the datafile contains no factor, its y-value is stored in
> place of the factor identifier, leading to ambiguities.
> If the datafile contains
> 0	0	0	foo
> 0	1
> The second row will be marked as part of the 'foo' boxplot.
> 
> This patch fixes that issue.
> ---
> diff -Naur gnuplot-5.0.0.orig/src/gadgets.h gnuplot-5.0.0/src/gadgets.h
> --- gnuplot-5.0.0.orig/src/gadgets.h	2015-01-02 22:06:25.488402227 +0100
> +++ gnuplot-5.0.0/src/gadgets.h	2015-01-02 22:06:25.458403577 +0100
> @@ -279,6 +279,8 @@
>  	BOXPLOT_FACTOR_LABELS_X2
>  } t_boxplot_factor_labels;
>  
> +#define DEFAULT_BOXPLOT_FACTOR -1
> +
>  typedef struct boxplot_style {
>      int limit_type;	/* 0 = multiple of interquartile 1 = fraction of points */
>      double limit_value;
> diff -Naur gnuplot-5.0.0.orig/src/plot2d.c gnuplot-5.0.0/src/plot2d.c
> --- gnuplot-5.0.0.orig/src/plot2d.c	2015-01-02 22:06:25.488402227 +0100
> +++ gnuplot-5.0.0/src/plot2d.c	2015-01-02 22:08:57.411693594 +0100
> @@ -910,19 +910,22 @@
>  		store2d_point(current_plot, i++, v[0], v[1], v[0], v[0],
>  			      v[1] - v[2], v[1] + v[2], -1.0);
>  	    } else {
> -		double w;
> +		double ylow, w;
>  		if (current_plot->plot_style == CANDLESTICKS
>  		    || current_plot->plot_style == FINANCEBARS) {
>  		    int_warn(storetoken, "This plot style does not work with 1 or 2 cols. Setting to points");
>  		    current_plot->plot_style = POINTSTYLE;
> -		}
> +		} else if (current_plot->plot_style == BOXPLOT)
> +		    ylow = DEFAULT_BOXPLOT_FACTOR;
> +		else
> +		    ylow = v[1];
>  		if (current_plot->plot_smooth == SMOOTH_ACSPLINES)
>  		    w = 1.0;	/* Unit weights */
>  		else
>  		    w = -1.0;	/* Auto-width boxes in some styles */
>  		/* Set x/y high/low to exactly [x,y] */
>  		store2d_point(current_plot, i++, v[0], v[1], 
> -						 v[0], v[0], v[1], v[1], w);
> +						 v[0], v[0], ylow, v[1], w);
>  	    }
>  	    break;
>  
> @@ -1004,7 +1007,7 @@
>  
>  		case BOXPLOT:	/* x, y, width */
>  		    store2d_point(current_plot, i++, v[0], v[1], v[0]-v[2]/2., v[0]+v[2]/2.,
> -		    		  v[1], v[1], v[2]);
> +				  DEFAULT_BOXPLOT_FACTOR, v[1], v[2]);
>  		    break;
>  
>  #ifdef EAM_OBJECTS
> @@ -1513,7 +1516,7 @@
>      /* This can happen if the user specifies a non-existent column:
>       * fall back to single-boxplot mode */
>      if (!string)
> -	return 0;
> +	return DEFAULT_BOXPLOT_FACTOR;
>  
>      /* Remove the trailing garbage, quotes etc. from the string */ 
>      trimmed_string = df_parse_string_field(string);

Bug #2
I agree there may be a bug here, but I can't reproduce exactly
what you describe.  Could you please provide a full example with
both the input and the command used to plot?

If I understand the problem case correctly, a missing column in 
the input data is a failure in its own right and the point should 
be discarded before ever reaching your new code.
The actual result seems to depend on additional factors like 
"set datafile separator". But in cases where the point is not 
discarded I don't see it incorrectly added to some other category
 - instead the plot contains an extra boxplot column with a blank
label into which the problem points are placed. 
That's arguably a correct result, although it's not what I expected.