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

[SW-2628] Support Java Serialization of NullableDataFrameParams on H2OMOJOModel #2659

Merged
merged 7 commits into from Feb 2, 2022

Conversation

mn-mikke
Copy link
Collaborator

No description provided.

@mn-mikke mn-mikke closed this Nov 15, 2021
@mn-mikke mn-mikke reopened this Jan 27, 2022
@mn-mikke mn-mikke changed the title [SW-2628] Java serialization of H2OMOJOModel [SW-2628] Support Java Serialization of DataFrame NullableDataFrameParams on H2OMOJOModel Jan 28, 2022
@mn-mikke mn-mikke changed the title [SW-2628] Support Java Serialization of DataFrame NullableDataFrameParams on H2OMOJOModel [SW-2628] Support Java Serialization of NullableDataFrameParams on H2OMOJOModel Jan 28, 2022
@mn-mikke mn-mikke marked this pull request as ready for review January 28, 2022 18:24
Copy link
Contributor

@kanech kanech left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -39,6 +39,7 @@ import org.apache.spark.expose.Logging
import org.apache.spark.ml.Model
import org.apache.spark.sql.catalyst.expressions.GenericRowWithSchema
import org.apache.spark.sql.types._
import ai.h2o.sparkling.utils.DataFrameSerializationWrappers._
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the additional import needed here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, here for example. $(crossValidationMetricsSummary) returns DataFrameSerializationWrapper and imported implicit method toDataFrame will look after the conversion to DataFrame.

Copy link
Member

@krasinski krasinski left a comment

Choose a reason for hiding this comment

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

LGTM 👍

btw. I'm wondering if it could be possible to use some custom data structure instead of dataframes in the future for params

@mn-mikke
Copy link
Collaborator Author

mn-mikke commented Feb 2, 2022

I'm wondering if it could be possible to use some custom data structure instead of dataframes in the future for params.

DataFrames might be too big gun for such a small task. I went for data frames since binding for Python and R is for free and it's API is well defined and well-known. I'm open to ideas for improvements, but we must be careful with breaking changes.

@mn-mikke mn-mikke merged commit e7fa632 into master Feb 2, 2022
@mn-mikke mn-mikke deleted the mn/SW-2628 branch February 2, 2022 15:02
mn-mikke added a commit that referenced this pull request Feb 2, 2022
…OMOJOModel (#2659)

* [SW-2628] Support Java Serialization of DataFrame NullableDataFrameParams on H2OMOJOModel

* Fixes after rebase

* Spotless apply

* Improve serialization tests

* fix python tests

* spotless apply

* Address review comments

(cherry picked from commit e7fa632)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants