Skip to content

Conversation

@ramanathanv
Copy link
Contributor

Hi,

I am creating this pull request that implements the basic methods in StringIndexer and StringIndexerModel class along with Test cases.

This is related to "Implement ML Features" #381

Comment on lines 58 to 59
/// <param name="source"></param>
/// <returns></returns>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please add the param and returns description?

@Niharikadutta
Copy link
Collaborator

@ramanathanv Is this PR ready to review? I see some StringIndexerTests tests failing in the pipeline.

@ramanathanv
Copy link
Contributor Author

@ramanathanv Is this PR ready to review? I see some StringIndexerTests tests failing in the pipeline.

Hi @Niharikadutta ,

In the test case, I am trying to compare two List objects using Assert.Equal(). I notice in the test results that the Lists have the same content (see image below). Still there is an error.
image

I assume that there must be a different way to compare the Lists. Can you please suggest me a way an alternate way to do this comparison?

Thanks.

@imback82
Copy link
Contributor

I notice in the test results that the Lists have the same content (see image below).

I see "..." in the output. Can you compare elements one by one to see where the difference comes from?

@ramanathanv
Copy link
Contributor Author

I notice in the test results that the Lists have the same content (see image below).

I see "..." in the output. Can you compare elements one by one to see where the difference comes from?

Hi @imback82 ,
I noticed that even the raw log file provides only 3 dots "...". Is there another way for me to check this?

@imback82
Copy link
Contributor

What I meant was if you are comparing two lists, you can compare elements in those lists one by one instead of comparing them at the list level. Does it make sense?

@GoEddie
Copy link
Contributor

GoEddie commented Feb 1, 2021

In StringIndexer there are a few calls which are missing the get* at the beginning - if I change them all to get* then the test passes for me.

public string GetStringOrderType() => (string)_jvmObject.Invoke("stringOrderType"); 

should be

public string GetStringOrderType() => (string)_jvmObject.Invoke("getStringOrderType"); 
public string[] GetInputCols() => (string[])_jvmObject.Invoke("inputCols"); 

should be

public string[] GetInputCols() => (string[])_jvmObject.Invoke("getInputCols"); 
public string GetInputCol() => (string)_jvmObject.Invoke("inputCol"); 

should be

public string GetInputCol() => (string)_jvmObject.Invoke("getInputCol"); 
public string GetHandleInvalid() => (string)_jvmObject.Invoke("handleInvalid"); 

should be

public string GetHandleInvalid() => (string)_jvmObject.Invoke("getHandleInvalid"); 

If you change those then the tests should pass :)

@ramanathanv
Copy link
Contributor Author

If you change those then the tests should pass :)

Thanks @GoEddie for identifying the issue . I have fixed the property names. But the test fails for unknown reasons. Can you please re-initiate the build/tests?

Base automatically changed from master to main March 18, 2021 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants