Re: conflict of separator and missing

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

On Wednesday, 15 June 2016 11:27:27 AM pl...@pi... wrote:
> On 15/06/16 00:06, Ethan A Merritt wrote:
> >> I think you are correct that there is a bug in this part of the code.
> >> > > The full-length string from 'set missing' is tested against the
> >> > > start of the field contents (after removing leading whitespace);
> >> > > then the subsequent character is tested to see if it is whitespace
> >> > > rather than a continuation of whatever string is in the field.
> >> > > So it works with a tab-separated *.csv file because <tab> counts
> >> > > as whitespace, but fails with a comma-separated file because the
> >> > > comma is mis-interpreted as part of the field content.
> >>
> 
> further thoughts:
> 
> The cause of this then, seems to be an oversight. The end of the string 
> is being tested as though it was the default case of WSpace separators 
> and not the specified separator.
> 
> It is a little irrelevant what name is give to this sort of file, the 
> key point is that the check you describe is not using the current 
> datafile separator.
> 
> Presumably the same thing would happen if someone had a file using colon 
> ( or any other non WS char ) as separator and had correctly specified it 
> with
> 
> 
> set datafile separator ":"
> 
> I have not tested this explicitly but there is nothing special about 
> using comma sep. so I presume the same bug would manifest.
> 
> " the subsequent character is tested to see if it is whitespace"
> 
> It seems that this test should be firstly a test for 'separator' and 
> then additionally for white-space + separator. As previously stated 
> "ignore A" probably should count as a match. Substrings counting as a 
> match is not described anywhere and I see not reason for this to be 
> taken as a hit.
> 
> Thanks for looking into this.
> Peter.

I have made a change to datafile.c:check_missing() in CVS for both 5.0 and 5.1.
In the case of a csv file (i.e. "set datafile separator" is non-blank) it now checks
for a match of the field contents to the "missing" string and requires that the
next character is a field-terminator.

Notes:
-   Leading whitespace is ignore but trailing whitespace is not.  
-   This is a obviously a change, so possibly there are existing scripts that break.
-   The comparison is to a string, not a numerical value, so -99.00 ne -99.0 ne -99   
-   If the "missing" string is quoted in the data file it will not be recognized.
-   If the "missing" string itself contains quotes, the behaviour is not specified

This change does not include an earlier suggestion to provide an option that
causes NaN (not-a-number) values to be treated as missing data.
I am inclined to add this also, probably as a new keyword  "set datafile missing NaN". 
See `help missing` for detail on the current handling of missing and NaN values.

	Ethan