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 data frame support #2

Merged

Conversation

weicongs-amazon
Copy link
Contributor

@weicongs-amazon weicongs-amazon commented Apr 13, 2021

Description of Changes

  • Add DataFrame related interface implementations. This is data interface used for data exchange between client plugins and ml plugin. Serialization and Deserialization also leverage the existing solution of transport actions.
  • Change the UT coverage to 90%. (Open for discussion)

Testing

  • UT: ./gradlew build

Copy link

@zhanghg08 zhanghg08 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for the changes.

Comment on lines +25 to +39
default int intValue() {
throw new RuntimeException("the value isn't Integer type");
}

default String stringValue() {
throw new RuntimeException("the value isn't String type");
}

default double doubleValue() {
throw new RuntimeException("the value isn't Double type");
}

default boolean booleanValue() {
throw new RuntimeException("the value isn't Boolean type");
}

Choose a reason for hiding this comment

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

Like the pattern here!

}

@Test
public void read_StringValue() throws IOException {
Copy link
Member

Choose a reason for hiding this comment

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

Consider beefing up tests for empty strings here, and NULL values here/elsewhere where appropriate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Add empty string check here. Null value isn't allowed for StringValue. Instead, add null check in StringValue UT.

Copy link
Member

Choose a reason for hiding this comment

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

🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants