Skip to content

Replace DvDateTimeZone, DvDateTime, DvTimeSpan with .NET standard types. #693

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 24 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
bc530e9
Replace date/time types with .NET standard types.
codemzs Aug 19, 2018
e0d66b0
rename DateTimeZone to DateTimeOffset.
codemzs Aug 19, 2018
54145da
Merge branch 'master' of https://github.com/dotnet/machinelearning in…
codemzs Aug 27, 2018
e55c780
PR feedback.
codemzs Aug 27, 2018
53a84d1
PR feedback.
codemzs Aug 27, 2018
b7d4c6b
PR feedback.
codemzs Aug 27, 2018
844da7c
Merge branch 'master' of https://github.com/dotnet/machinelearning in…
codemzs Aug 28, 2018
dbaac06
IDV test.
codemzs Aug 28, 2018
53b4e57
add test for parquet loader.
codemzs Aug 28, 2018
ae9aede
Merge branch 'parquettest' of https://github.com/codemzs/machinelearn…
codemzs Aug 28, 2018
48b4c08
Update parquet tests.
codemzs Aug 28, 2018
7f55c59
fix build.
codemzs Aug 28, 2018
71f8701
PR feedback.
codemzs Aug 28, 2018
1ccc263
clean up.
codemzs Aug 29, 2018
1672739
Merge branch 'master' of https://github.com/dotnet/machinelearning in…
codemzs Aug 29, 2018
5231b51
Merge branch 'master' of https://github.com/dotnet/machinelearning in…
codemzs Aug 30, 2018
0a1ba05
PR feedback.
codemzs Aug 30, 2018
0560602
Merge branch 'master' of https://github.com/dotnet/machinelearning in…
codemzs Aug 30, 2018
401fa64
rebuild.
codemzs Aug 30, 2018
c3a5a2d
Merge branch 'master' of https://github.com/dotnet/machinelearning in…
codemzs Aug 31, 2018
74a652c
Merge branch 'master' of https://github.com/dotnet/machinelearning in…
codemzs Sep 4, 2018
7cc7d9a
merge parquet test.
codemzs Sep 4, 2018
a42861c
Merge branch 'master' of https://github.com/dotnet/machinelearning in…
codemzs Sep 7, 2018
ea943d6
merge master.
codemzs Sep 7, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Microsoft.ML.Api/ApiUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ private static OpCode GetAssignmentOpCode(Type t)
t == typeof(DvBool) || t == typeof(DvText) || t == typeof(string) || t.IsArray ||
(t.IsGenericType && t.GetGenericTypeDefinition() == typeof(VBuffer<>)) ||
(t.IsGenericType && t.GetGenericTypeDefinition() == typeof(Nullable<>)) ||
t == typeof(DvDateTime) || t == typeof(DvDateTimeZone) || t == typeof(DvTimeSpan) || t == typeof(UInt128))
t == typeof(DateTime) || t == typeof(DateTimeOffset) || t == typeof(TimeSpan) || t == typeof(UInt128))
{
return OpCodes.Stobj;
}
Expand Down
30 changes: 15 additions & 15 deletions src/Microsoft.ML.Core/Data/ColumnType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ public bool IsTimeSpan
}

/// <summary>
/// Whether this type is a DvDateTime.
/// Whether this type is a DateTime.
Copy link
Contributor

@TomFinley TomFinley Aug 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DateTime [](start = 35, length = 8)

<see tags perhaps #Resolved

/// </summary>
public bool IsDateTime
{
Expand All @@ -150,16 +150,16 @@ public bool IsDateTime
}

/// <summary>
/// Whether this type is a DvDateTimeZone.
/// Whether this type is a DateTimeOffset.
/// </summary>
public bool IsDateTimeZone
{
get
{
if (!(this is DateTimeZoneType))
if (!(this is DateTimeOffsetType))
return false;
// DateTimeZoneType is a singleton.
Contracts.Assert(this == DateTimeZoneType.Instance);
// DateTimeOffsetType is a singleton.
Contracts.Assert(this == DateTimeOffsetType.Instance);
return true;
}
}
Expand Down Expand Up @@ -319,7 +319,7 @@ public static PrimitiveType FromKind(DataKind kind)
if (kind == DataKind.DT)
return DateTimeType.Instance;
if (kind == DataKind.DZ)
return DateTimeZoneType.Instance;
return DateTimeOffsetType.Instance;
return NumberType.FromKind(kind);
}
}
Expand Down Expand Up @@ -605,7 +605,7 @@ public static DateTimeType Instance
}

private DateTimeType()
: base(typeof(DvDateTime), DataKind.DT)
: base(typeof(DateTime), DataKind.DT)
{
}

Expand All @@ -623,29 +623,29 @@ public override string ToString()
}
}

public sealed class DateTimeZoneType : PrimitiveType
public sealed class DateTimeOffsetType : PrimitiveType
{
private static volatile DateTimeZoneType _instance;
public static DateTimeZoneType Instance
private static volatile DateTimeOffsetType _instance;
public static DateTimeOffsetType Instance
{
get
{
if (_instance == null)
Interlocked.CompareExchange(ref _instance, new DateTimeZoneType(), null);
Interlocked.CompareExchange(ref _instance, new DateTimeOffsetType(), null);
return _instance;
}
}

private DateTimeZoneType()
: base(typeof(DvDateTimeZone), DataKind.DZ)
private DateTimeOffsetType()
: base(typeof(DateTimeOffset), DataKind.DZ)
{
}

public override bool Equals(ColumnType other)
{
if (other == this)
return true;
Contracts.Assert(!(other is DateTimeZoneType));
Contracts.Assert(!(other is DateTimeOffsetType));
return false;
}

Expand All @@ -672,7 +672,7 @@ public static TimeSpanType Instance
}

private TimeSpanType()
: base(typeof(DvTimeSpan), DataKind.TS)
: base(typeof(TimeSpan), DataKind.TS)
{
}

Expand Down
12 changes: 6 additions & 6 deletions src/Microsoft.ML.Core/Data/DataKind.cs
Original file line number Diff line number Diff line change
Expand Up @@ -165,11 +165,11 @@ public static Type ToType(this DataKind kind)
case DataKind.BL:
return typeof(DvBool);
case DataKind.TS:
return typeof(DvTimeSpan);
return typeof(TimeSpan);
case DataKind.DT:
return typeof(DvDateTime);
return typeof(DateTime);
case DataKind.DZ:
return typeof(DvDateTimeZone);
return typeof(DateTimeOffset);
case DataKind.UG:
return typeof(UInt128);
}
Expand Down Expand Up @@ -209,11 +209,11 @@ public static bool TryGetDataKind(this Type type, out DataKind kind)
kind = DataKind.TX;
else if (type == typeof(DvBool) || type == typeof(bool) || type == typeof(bool?))
kind = DataKind.BL;
else if (type == typeof(DvTimeSpan))
else if (type == typeof(TimeSpan))
kind = DataKind.TS;
else if (type == typeof(DvDateTime))
else if (type == typeof(DateTime))
kind = DataKind.DT;
else if (type == typeof(DvDateTimeZone))
else if (type == typeof(DateTimeOffset))
kind = DataKind.DZ;
else if (type == typeof(UInt128))
kind = DataKind.UG;
Expand Down
97 changes: 22 additions & 75 deletions src/Microsoft.ML.Data/Data/Conversion.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
namespace Microsoft.ML.Runtime.Data.Conversion
{
using BL = DvBool;
using DT = DvDateTime;
using DZ = DvDateTimeZone;
using DT = DateTime;
using DZ = DateTimeOffset;
using I1 = DvInt1;
using I2 = DvInt2;
using I4 = DvInt4;
Expand All @@ -28,7 +28,7 @@ namespace Microsoft.ML.Runtime.Data.Conversion
using RawI4 = Int32;
using RawI8 = Int64;
using SB = StringBuilder;
using TS = DvTimeSpan;
using TS = TimeSpan;
using TX = DvText;
using U1 = Byte;
using U2 = UInt16;
Expand Down Expand Up @@ -252,9 +252,6 @@ private Conversions()
AddIsNA<R8>(IsNA);
AddIsNA<BL>(IsNA);
AddIsNA<TX>(IsNA);
AddIsNA<TS>(IsNA);
AddIsNA<DT>(IsNA);
AddIsNA<DZ>(IsNA);

AddGetNA<I1>(GetNA);
AddGetNA<I2>(GetNA);
Expand All @@ -264,9 +261,6 @@ private Conversions()
AddGetNA<R8>(GetNA);
AddGetNA<BL>(GetNA);
AddGetNA<TX>(GetNA);
AddGetNA<TS>(GetNA);
AddGetNA<DT>(GetNA);
AddGetNA<DZ>(GetNA);

AddHasNA<I1>(HasNA);
AddHasNA<I2>(HasNA);
Expand All @@ -276,9 +270,6 @@ private Conversions()
AddHasNA<R8>(HasNA);
AddHasNA<BL>(HasNA);
AddHasNA<TX>(HasNA);
AddHasNA<TS>(HasNA);
AddHasNA<DT>(HasNA);
AddHasNA<DZ>(HasNA);

AddIsDef<I1>(IsDefault);
AddIsDef<I2>(IsDefault);
Expand Down Expand Up @@ -853,9 +844,6 @@ public ValueGetter<T> GetNAOrDefaultGetter<T>(ColumnType type)
private bool IsNA(ref R4 src) => src.IsNA();
private bool IsNA(ref R8 src) => src.IsNA();
private bool IsNA(ref BL src) => src.IsNA;
private bool IsNA(ref TS src) => src.IsNA;
private bool IsNA(ref DT src) => src.IsNA;
private bool IsNA(ref DZ src) => src.IsNA;
private bool IsNA(ref TX src) => src.IsNA;
#endregion IsNA

Expand All @@ -867,9 +855,6 @@ public ValueGetter<T> GetNAOrDefaultGetter<T>(ColumnType type)
private bool HasNA(ref VBuffer<R4> src) { for (int i = 0; i < src.Count; i++) { if (src.Values[i].IsNA()) return true; } return false; }
private bool HasNA(ref VBuffer<R8> src) { for (int i = 0; i < src.Count; i++) { if (src.Values[i].IsNA()) return true; } return false; }
private bool HasNA(ref VBuffer<BL> src) { for (int i = 0; i < src.Count; i++) { if (src.Values[i].IsNA) return true; } return false; }
private bool HasNA(ref VBuffer<TS> src) { for (int i = 0; i < src.Count; i++) { if (src.Values[i].IsNA) return true; } return false; }
private bool HasNA(ref VBuffer<DT> src) { for (int i = 0; i < src.Count; i++) { if (src.Values[i].IsNA) return true; } return false; }
private bool HasNA(ref VBuffer<DZ> src) { for (int i = 0; i < src.Count; i++) { if (src.Values[i].IsNA) return true; } return false; }
private bool HasNA(ref VBuffer<TX> src) { for (int i = 0; i < src.Count; i++) { if (src.Values[i].IsNA) return true; } return false; }
#endregion HasNA

Expand Down Expand Up @@ -907,9 +892,6 @@ public ValueGetter<T> GetNAOrDefaultGetter<T>(ColumnType type)
private void GetNA(ref R4 value) => value = R4.NaN;
private void GetNA(ref R8 value) => value = R8.NaN;
private void GetNA(ref BL value) => value = BL.NA;
private void GetNA(ref TS value) => value = TS.NA;
private void GetNA(ref DT value) => value = DT.NA;
private void GetNA(ref DZ value) => value = DZ.NA;
private void GetNA(ref TX value) => value = TX.NA;
#endregion GetNA

Expand Down Expand Up @@ -1041,9 +1023,9 @@ public void Convert(ref BL src, ref SB dst)
else if (src.IsTrue)
dst.Append("1");
}
public void Convert(ref TS src, ref SB dst) { ClearDst(ref dst); if (!src.IsNA) dst.AppendFormat("{0:c}", (TimeSpan)src); }
public void Convert(ref DT src, ref SB dst) { ClearDst(ref dst); if (!src.IsNA) dst.AppendFormat("{0:o}", (DateTime)src); }
public void Convert(ref DZ src, ref SB dst) { ClearDst(ref dst); if (!src.IsNA) dst.AppendFormat("{0:o}", (DateTimeOffset)src); }
public void Convert(ref TS src, ref SB dst) { ClearDst(ref dst); dst.AppendFormat("{0:c}", src); }
public void Convert(ref DT src, ref SB dst) { ClearDst(ref dst); dst.AppendFormat("{0:o}", src); }
public void Convert(ref DZ src, ref SB dst) { ClearDst(ref dst); dst.AppendFormat("{0:o}", src); }
#endregion ToStringBuilder

#region FromR4
Expand Down Expand Up @@ -1472,61 +1454,37 @@ public bool TryParse(ref TX src, out R8 dst)

public bool TryParse(ref TX src, out TS dst)
{
dst = default;
if (!src.HasChars)
{
if (src.IsNA)
dst = TS.NA;
else
dst = default(TS);
return true;
}
TimeSpan res;
if (TimeSpan.TryParse(src.ToString(), CultureInfo.InvariantCulture, out res))
{
dst = new TS(res);

if (TimeSpan.TryParse(src.ToString(), CultureInfo.InvariantCulture, out dst))
return true;
}
dst = TS.NA;

return IsStdMissing(ref src);
}

public bool TryParse(ref TX src, out DT dst)
{
dst = default;
if (!src.HasChars)
{
if (src.IsNA)
dst = DvDateTime.NA;
else
dst = default(DvDateTime);
return true;
}
DateTime res;
if (DateTime.TryParse(src.ToString(), CultureInfo.InvariantCulture, DateTimeStyles.AssumeUniversal | DateTimeStyles.AdjustToUniversal, out res))
{
dst = new DT(res);

if (DateTime.TryParse(src.ToString(), CultureInfo.InvariantCulture, DateTimeStyles.AssumeUniversal | DateTimeStyles.AdjustToUniversal, out dst))
return true;
}
dst = DvDateTime.NA;

return IsStdMissing(ref src);
Copy link
Contributor

@TomFinley TomFinley Aug 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IsStdMissing(ref src); [](start = 19, length = 22)

So I feel like if you parse missing into a value type that does not support missing, we might expect the result to be that we would throw, rather than just the default value. #Resolved

}

public bool TryParse(ref TX src, out DZ dst)
{
dst = default;
if (!src.HasChars)
{
if (src.IsNA)
dst = DvDateTimeZone.NA;
else
dst = default(DvDateTimeZone);
return true;
}
DateTimeOffset res;
if (DateTimeOffset.TryParse(src.ToString(), CultureInfo.InvariantCulture, DateTimeStyles.AssumeUniversal, out res))
{
dst = new DZ(res);

if (DateTimeOffset.TryParse(src.ToString(), CultureInfo.InvariantCulture, DateTimeStyles.AssumeUniversal, out dst))
return true;
}
dst = DvDateTimeZone.NA;

return IsStdMissing(ref src);
Copy link
Contributor

@TomFinley TomFinley Aug 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IsStdMissing [](start = 19, length = 12)

Similar for this. #Resolved

}

Expand Down Expand Up @@ -1804,21 +1762,10 @@ public void Convert(ref TX src, ref SB dst)
src.AddToStringBuilder(dst);
}

public void Convert(ref TX span, ref TS value)
{
if (!TryParse(ref span, out value))
Contracts.Assert(value.IsNA);
}
public void Convert(ref TX span, ref DT value)
{
if (!TryParse(ref span, out value))
Contracts.Assert(value.IsNA);
}
public void Convert(ref TX span, ref DZ value)
{
if (!TryParse(ref span, out value))
Contracts.Assert(value.IsNA);
}
public void Convert(ref TX span, ref TS value) => TryParse(ref span, out value);
public void Convert(ref TX span, ref DT value) => TryParse(ref span, out value);
public void Convert(ref TX span, ref DZ value) => TryParse(ref span, out value);

#endregion FromTX

#region FromBL
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.ML.Data/DataLoadSave/Binary/CodecFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ public CodecFactory(IHostEnvironment env, MemoryStreamPool memPool = null)
RegisterSimpleCodec(new UnsafeTypeCodec<ulong>(this));
RegisterSimpleCodec(new UnsafeTypeCodec<Single>(this));
RegisterSimpleCodec(new UnsafeTypeCodec<Double>(this));
RegisterSimpleCodec(new UnsafeTypeCodec<DvTimeSpan>(this));
RegisterSimpleCodec(new UnsafeTypeCodec<TimeSpan>(this));
Copy link
Contributor

@Ivanidzo4ka Ivanidzo4ka Aug 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UnsafeTypeCodec [](start = 36, length = 15)

Is it still need to be UnsafeTypeCodec? #Resolved

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What should it be? I have defined TimeSpanUnsafeTypeOps to handle this type.


In reply to: 211110052 [](ancestors = 211110052)

RegisterSimpleCodec(new DvTextCodec(this));
RegisterSimpleCodec(new BoolCodec(this));
RegisterSimpleCodec(new DateTimeCodec(this));
RegisterSimpleCodec(new DateTimeZoneCodec(this));
RegisterSimpleCodec(new DateTimeOffsetCodec(this));
RegisterSimpleCodec(new UnsafeTypeCodec<UInt128>(this));

// Register the old boolean reading codec.
Expand Down
Loading