Skip to content
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

Add missing KeyType documentation #2783

Merged
merged 6 commits into from
Mar 12, 2019
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/Microsoft.ML.Core/Data/KeyType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ public static bool IsValidDataType(Type type)
/// </summary>
public ulong Count { get; }

/// <summary>
/// Determine if a DataViewType object is equal to another KeyType instance.
Copy link
Contributor

Choose a reason for hiding this comment

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

DataViewType [](start = 27, length = 12)

We might prefer <see tags. Not that I think we'll rename them this late in the game, but the links would probably be nice.

Copy link
Contributor

Choose a reason for hiding this comment

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

KeyType [](start = 67, length = 7)

I might reverse this, so, "if this KeyType instance is equal to another DataViewType instance." SInce this is assuredly a key type, and the other thing is assuredly a DataView Type. Note also, I might suggest being consistent in this sentence between the usage of instance and object. And the use of "this" instead of "a". Anyway, these latter points are not really big deals.

/// Checks if the other item is the type of KeyType, if the RawType
/// is the same, and if the Count is the same.
Copy link
Contributor

Choose a reason for hiding this comment

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

Count [](start = 36, length = 5)

Similar notes on <see tags, here and everywhere.

/// </summary>
/// <param name="other">The other object to compare against.</param>
/// <returns>A boolean if both objects are equal or not.</returns>
public override bool Equals(DataViewType other)
{
if (other == this)
Expand All @@ -73,16 +80,31 @@ public override bool Equals(DataViewType other)
return true;
}

/// <summary>
/// Determine if a KeyType instance is equal to another KeyType instance.
/// Checks if any object is the type of KeyType, if the RawType
/// is the same, and if the Count is the same.
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you use the <see tag for Count and for RawType?

/// </summary>
/// <param name="other">The other object to compare against.</param>
/// <returns>A boolean if both objects are equal or not.</returns>
Copy link
Contributor

Choose a reason for hiding this comment

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

/// A boolean if both objects are equal or not.</returns [](start = 7, length = 66)

for bool returns, please document them like this:

/// <see langword="true" /> to do averaging; otherwise, <see langword="false" />.
#Resolved

public override bool Equals(object other)
{
return other is DataViewType tmp && Equals(tmp);
}

/// <summary>
/// Retrieves the hash code.
/// </summary>
/// <returns>An integer representing the hash code.</returns>
public override int GetHashCode()
{
return Hashing.CombinedHash(RawType.GetHashCode(), Count);
}

/// <summary>
/// The string representation of the KeyType.
/// </summary>
/// <returns>A formatted string.</returns>
public override string ToString()
{
InternalDataKind rawKind = this.GetRawKind();
Expand Down