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

rankerType got from options in QnA constructor #1953

Merged
merged 6 commits into from
Apr 8, 2020
Merged

rankerType got from options in QnA constructor #1953

merged 6 commits into from
Apr 8, 2020

Conversation

davidalcoba
Copy link
Contributor

Fixes #1951

Description

rankerType option passed as option parameter in QnA constructor was ignored

Specific Changes

  • rankerType got from options parameters in QnA constructor
  • unit test 'should call qnamaker with rankerType questionOnly' updated

Testing

  • no tests added, only above mentioned test has been updated

@msftclas
Copy link

msftclas commented Mar 26, 2020

CLA assistant check
All CLA requirements met.

Copy link
Contributor

@vipeketi vipeketi left a comment

Choose a reason for hiding this comment

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

The score value should be in range of 0 and 1
/**
* Confidence on a scale from 0.0 to 1.0 that the answer matches the users intent.
*/
score: number;

@davidalcoba
Copy link
Contributor Author

Thanks @vipeketi for your comment.

Not sure if I understand you, do you refer to the unit test response json? If so, I have to accept that I couldn't test the real qna service after providing the rankerType option because I assume that the qna api keys are not published. So I opted to modify directly the mock.

If someone could provide the real answer from qna service for the updated unit test, I will adapt the mock.

@vipeketi
Copy link
Contributor

vipeketi commented Mar 27, 2020 via email

Copy link
Contributor

@vipeketi vipeketi left a comment

Choose a reason for hiding this comment

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

Rankertype is set now.

Copy link
Contributor

@vipeketi vipeketi left a comment

Choose a reason for hiding this comment

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

Approved

@davidalcoba
Copy link
Contributor Author

This pull request can be automatically merged by project collaborators
Only those with write access to this repository can merge pull requests.

Who can do that?

@vipeketi vipeketi merged commit d54da72 into microsoft:master Apr 8, 2020
@vipeketi
Copy link
Contributor

vipeketi commented Apr 8, 2020

Merged changes.

@davidalcoba davidalcoba deleted the fix/rankerType-options-left branch April 9, 2020 07:12
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.

rankerType options left
3 participants