Change TranslationRequest into something like TranslationModelConfig

[browsermt]

Change TranslationRequest into something like TranslationModelConfig

[motin]

Adding a yaml/json-formatted string as a parameter to TranslationRequest for specifying options specific to the request is definitely possible/performant in JS.

Note that while I am not married to the current design of the API/bindings and have no issues with updating the integration code to match whatever is decided in this repo, neither am I actively involved in the design decisions of the API at the moment, so I'll let y'all come up with the changes you need to achieve progress on that front, if that is ok with you?

[motin]

@abhi-agg ping for comments

[abhi-agg]

@jerinphilip If you are proposing to remove TranslationRequest class altogether and pass the translation request parameters as a YAML/JSON formatted std::string then I am fine with it.

I have just one comment. Since, model configuration is already being provided as a yaml-formatted string, it would be consistent if we treat the translation request also as a yaml-formatted string (instead of json formatted string). Is it fine with you?

We will have to add a documentation listing all the keys and the corresponding values that can be provided as translation request. Could you document them here? The PR review will be straightforward then 👍

[jerinphilip]

All I have asked is this structure have certain properties similar to a dict. YAML or JSON is just a conversion thing (also aren't they same except how they're printed?), and the redundancies can be removed/adjusted during implementation. A string representation of such a dict is parsable into the dict, which suffices for me.

Could you document them here?

Probably not, this issue is solved at typedef std::string TranslationRequest;, let's have it at another issue.

[abhi-agg]

YAML or JSON is just a conversion thing (also aren't they same except how they're printed?)

They are not same. YAML is a superset of JSON => A YAML parser can parse both JSON and a YAML formatted string.

Could you document them here?

Probably not, this issue is solved at typedef std::string TranslationRequest;,

Probably, I am not getting what you meant here. Let me ask you this way. What should we provide as std::string for translation request?

let's have it at another issue.

Wait. The title says "Change TranslationRequest into something like TranslationModelConfig". Is this issue not the right place for this discussion?

[jerinphilip]

@abhi-agg Scope is bigger, moving to #89. Resolution, as I understand here, is you're okay dropping the existing TranslationRequest class (effectively replacing it with YAML string). I'll leave the issue open until the implementation is complete.