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

modify recommendation assertion. #2

Open
wants to merge 36 commits into
base: keerthiyanda/spark3.4
Choose a base branch
from

Conversation

JessicaXYWang
Copy link

No description provided.

svotaw and others added 5 commits August 28, 2023 19:28
@JessicaXYWang
Copy link
Author

JessicaXYWang commented Aug 31, 2023

@BrendanWalsh Hi Brendan, can you help to review it? Thanks.

@@ -24,4 +39,18 @@ class RankingAdapterModelSpec extends RankingTestBase with TransformerFuzzing[Ra
}

override def reader: MLReadable[_] = RankingAdapterModel

override def assertDFEq(df1: DataFrame, df2: DataFrame)(implicit eq: Equality[DataFrame]): Unit = {

Choose a reason for hiding this comment

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

unless i'm missing something this is the same as above. can we reduce code duplication?

@@ -15,6 +17,19 @@ class RankingAdapterSpec extends RankingTestBase with EstimatorFuzzing[RankingAd

override def modelReader: MLReadable[_] = RankingAdapterModel

override def assertDFEq(df1: DataFrame, df2: DataFrame)(implicit eq: Equality[DataFrame]): Unit = {
def prep(df: DataFrame) = {
// sort rows and round decimals before compare two dataframes

Choose a reason for hiding this comment

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

With the goal of making sure that we're not hiding a real issue by changing tests, are we sure it's accurate to sort and round?
Why did it work before when it was unsorted and unrounded?

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.

4 participants