bugPSPP - Bugs: bug #17424, FREQUENCIES doesn't properly parse...

 
 

bug #17424: FREQUENCIES doesn't properly parse percentiles, ntiles

Submitter:  Ben Pfaff <blp>
Submitted:  Mon 14 Aug 2006 01:33:22 AM UTC
   
 
Category:  Syntax Parser Severity:  3 - Ordinary
Status:  Fixed Assigned to:  blp
Open/Closed:  Closed Release:  Before 0.6.0
Effort:  0.00
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 02 Aug 2007 03:09:16 AM UTC, comment #7: 

I checked this in.  Thanks again.

Ben Pfaff <blp>
Group administrator
Wed 01 Aug 2007 06:15:41 AM UTC, comment #6: 

Looks good to me.

John Darrington <jmd>
Group administrator
Wed 01 Aug 2007 04:16:54 AM UTC, comment #5: 

Thank you for your review.  You're right, my patch was really half-baked.  I can't believe I didn't run "make check"--I must have just tried the single test case in the bug report.

I'm attaching a revised version that passes "make check".  Do you like it better?

(file #13541)

Ben Pfaff <blp>
Group administrator
Tue 31 Jul 2007 12:14:45 AM UTC, comment #4: 

This patch certainly makes the code look cleaner.

However after applying it, I get 3 test failures.  Two of which seem to be trivial, but the other (in tests/command/weight.sh) appears to be a numerical discrepancy:

77c77
< |Median           |     .000|
---

> |Median           |   28.500|

compare results
FAILED
FAIL: tests/command/weight.sh

I think you need to change frequencies.q:364 to read

 if (cmd.a_statistics[FRQ_ST_MEDIAN] ||  cmd.a_statistics[FRQ_ST_ALL])

There may be other subtle interactions too.  I haven't considered everything in detail ...

John Darrington <jmd>
Group administrator
Mon 30 Jul 2007 10:47:16 PM UTC, comment #3: 

BTW the patch is meant to apply atop the one for bug #17421.

Ben Pfaff <blp>
Group administrator
Mon 30 Jul 2007 10:40:34 PM UTC, comment #2: 

The attached patch fixes this bug.  Turned out that calc_stats was always adding a 50th percentile to the list of percentiles (whether we asked for the median or not) if it wasn't already present.  In turn, dump_statistics was dropping the last percentile (not the 50th percentile) from display if calc_stats had added a 50th percentile.

I decided that we should just treat the median as the 50th percentile, and then display it in the output table as "50 (Median)".  I think that this is reasonable behavior.  It gets rid of a special case while fixing the bug.

Comments?

(file #13530)

Ben Pfaff <blp>
Group administrator
Mon 30 Jul 2007 05:11:25 PM UTC, comment #1: 

I'm working on FREQUENCIES.

Ben Pfaff <blp>
Group administrator
Mon 14 Aug 2006 01:33:22 AM UTC, original submission:  

According to the reporter:

In the FREQUENCIES command  : centiles above 50 are calculated ONLY
if NTILES subcommand is written before PERCENTILES subcommand:


Command file #1 :

****************************************************************


DATA LIST NOTABLE FILE=exemple44 /X1 8 X2 10-11.
VARIABLE LABELS X1 "Sexe" X2 "Age".
VALUE LABELS X1 1 "Homme" 2 "Femme".
FREQUENCIES X2 /FORMAT=NOTABLE /STAT=MODE /PERCENTILES=10,90.

****************************************************************


Result #1 :


****************************************************************

1.1 FREQUENCIES.  X2: Age
+-------------------+------+
|N           Valid  |    10|
|            Missing|     0|
|Mean               |32.900|
|Mode               |35.000|
|Percentiles 10     |21.000|
|            50     |35.000|
+-------------------+------+

****************************************************************

Command file #2 :

****************************************************************

DATA LIST NOTABLE FILE=exemple44 /X1 8 X2 10-11.
VARIABLE LABELS X1 "Sexe" X2 "Age".
VALUE LABELS X1 1 "Homme" 2 "Femme".
FREQUENCIES X2 /FORMAT=NOTABLE /STAT=MODE /NTILES=4 /PERCENTILES=10,90.
****************************************************************

Result #2 :


****************************************************************

+-------------------+------+
|N           Valid  |    10|
|            Missing|     0|
|Mean               |32.900|
|Mode               |35.000|
|Percentiles 0      |21.000|
|            10     |21.000|
|            25     |23.000|
|            50     |35.000|
|            75     |38.750|
|            90     |43.800|
|            100    |51.000|
+-------------------+------+

Ben Pfaff <blp>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jmd (Posted a comment)
  • -email is unavailable- added by blp (Posted a comment)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2007-08-02 blp StatusReady for Test/Review Fixed
        Open/ClosedOpen Closed
    2007-08-01 blp Attached File- Added frequencies-fix-percentiles.patch, #13541
    2007-07-30 blp Attached File- Added frequencies-fix-percentiles.patch, #13530
        StatusIn Progress Ready for Test/Review
    2007-07-30 blp StatusNone In Progress
    2006-08-14 blp ReleaseNone Before 0.6.0
    2006-08-14 blp Carbon-Copy- Added josephsaintpierre<joseph.saint-pierre@cict.fr>

    Back to the top

    Powered by Savane 3.15.
    Corresponding source code