konne / RserveCLI2

A fork of RServeCLI
Other
30 stars 21 forks source link

SexpArrayDate doesn't handle NAs well #8

Open SurajGupta opened 11 years ago

SurajGupta commented 11 years ago

AsDate/AsDates will return System.ArgumentOutOfRangeException

Yusuf90 commented 5 years ago

Currently SexpArrayDate uses Value Type DateTime to parse Date type Sexp's. DateTime (and integer) isn't nullable, therefore translating to something that looks like NA wouldn't adhere to C# principles.

Now I've seen a solution for SexpArrayInt NA's, and that's a special value that marks an integer as NA (which is -2147483648). In order to fix this issue, we need to decide on a special value for DateTime as well.

In the case of XtArrayDouble in Qap1.cs's DecodeSexp, you need to check if there is a NaN double value.

case XtArrayDouble:
{
    var res = new double[ length / 8 ];
    var doubleBuf = new byte[ 8 ];
    for ( int i = 0 ; i < length ; i += 8 )
    {
        Array.Copy( data , start + i , doubleBuf , 0 , 8 );
        res[ i / 8 ] = BitConverter.ToDouble( doubleBuf , 0 );
    }

    // is date or just a double?
    if ( ( attrs != null ) && ( attrs.ContainsKey( "class" ) && attrs[ "class" ].AsStrings.Contains( "Date" ) ) )
    {
        //result = new SexpArrayDate( res.Select( Convert.ToInt32 ) );
        int[] tempBuf = new int[res.Length];
        for (int i = 0; i < res.Length; ++i)
        {
            if (double.IsNaN(res[i]))
            {
                tempBuf[i] = SexpArrayInt.Na;
            }
            else
            {
                tempBuf[i] = Convert.ToInt32(res[i]);
            }
        }
        result = new SexpArrayDate(tempBuf);
    }
    else
    {
        result = new SexpArrayDouble( res );
    }
}
break;

In SexpArrayDate.cs's RIntToDate, we can decide on a special value if SexpArrayInt.Na is found. For completeness, I simply add zero days.

    private static DateTime RIntToDate( int rdate )
    {
        if (rdate == Na)
        {
            return Origin.AddDays(0); // special value
        }
        return Origin.AddDays( rdate );
    }

It would be pretty interesting if we could discuss and pick this 'special' value. One other solution would also be to use a nullable DateTime (DateTime?).

I am pretty new to GitHub, just wanted to drop my 2 cents here since I am pretty invested in the library over the past months.

Yusuf90 commented 5 years ago

I have decided that DateTime.MaxValue can be a NA value for DateTimes. #64