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

DCR: As a developer I would like to be able to pass LuisOptions to RecognizeAsync so I can override some settings based on the current user #5405

Closed
6 tasks done
gabog opened this issue May 29, 2019 · 3 comments
Assignees
Labels
P1 Painful if we don't fix, won't block releasing

Comments

@gabog
Copy link
Contributor

gabog commented May 29, 2019

Issue

In many cases, we declare LuisRecognizer as a singleton in startup so we can reuse it. LuisRecognizer takes a LuisPredictionOptions parameter in the constructor and the developer can't change the values after the object has been created.

There are several properties in LuisOptions that a developer should be able to change when invoking RecognizeAsync:

  • TimezoneOffset: this is used by LUIS to determine relative times (i.e.: 2 hours from now, tomorrow, next tuesday, etc.), it should be possible to set this value based on the user's timezone when calling RecognizeAsync.
  • SpellCheck: in some special situations, a developer may want to disable spell check for the utterance, for example, I worked with a travel agency where some record locators where close to english words and they word autocorrected resulting on the wrong entity being returned.
  • Log and LogPersonalInformation: A developer may want to disable logging for a particular request based on the user's privacy settings.

I don't have concrete use cases but I think the following parameters can be also overridden per request:

  • IncludeAllIntents
  • Staging
  • IncludeInstanceData

Here is an example of the private implementation of RecognizeAsync where some of these options are being used:

image

Proposed change

I think the easiest solution is to add an override for RecognizeAsync that takes LuisPredictionOptions and overrides the values passed in the constructor with the new ones passed in the request. If the optional parameter is passed and a given property is set, the recognizer should use it, if not, it should use whatever was set in the object passed in the constructor (most properties in LuisPredictionOptions are nullable or have defaults defined so this should be doable)

Component Impact

  • LuisRecognizer will be impacted as part of this DCR.

Customer Impact

This is an addition to the current class, existing users shouldn't be impacted by this change.

Tracking Status

Dotnet SDK PR 1992

  • PR
  • Merged

Javascript SDK JS Issue 964

  • PR
  • Merged

Python SDK Python issue 206

  • PR
  • Merged

[dcr]

@gabog
Copy link
Contributor Author

gabog commented May 29, 2019

@Stevenic, @johnataylor, @cleemullins: can you take a look at this DCR and let me know what you think?

@sgellock sgellock added 4.5 P1 Painful if we don't fix, won't block releasing labels May 29, 2019
@gabog
Copy link
Contributor Author

gabog commented Jun 3, 2019

C# PR created: microsoft/botbuilder-dotnet#1992

@sgellock
Copy link
Member

sgellock commented Jul 8, 2019

happiness has been achieved.

@sgellock sgellock closed this as completed Jul 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 Painful if we don't fix, won't block releasing
Projects
None yet
Development

No branches or pull requests

2 participants