-
Notifications
You must be signed in to change notification settings - Fork 38
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
HTML/Non-HTML translation per item of Batch Translation API #345
Comments
@abhi-agg This is a welcome change. Adding that we will also need to adapt My suggestion is to do this in two phases (a) Get the API change in - there will be some adjustments required for the bridge at tests for both service apps initially (b) Work out a cleaner place to place tests for the longer term (looking at https://github.com/browsermt/bergamot-translator/blob/34786520cd90a4fd3cb006a60648d57d4a5ed56c/src/tests/wasm.cpp). This would mean you get (a) immediately to experiment with at the extension, while the stability things come later. Let me know if this is acceptable, in which case I can bring the required changes shortly. |
@jerinphilip I agree with your proposal. Let's go ahead with it 👍🏾 Btw, do you have a timeline on when the change in the API could be available so that we can start experiment in the extension? I am asking this so that we can plan better for this feature at our end. |
Changes signature of BlockingService::{translate,pivot}Multiple functions to take per input options, so a mix of HTML and plaintext can be sent from the extension. Templating over testing is adjusted to allow for continuous evaluations by modifying the test code. Updates WebAssembly bindings to reflect the change in signature and the javascript test-page to work with the new bindings. This change lacks an accompanying test specific to the mixed HTML and plaintext inputs. Fixes: #345 See also: mozilla/firefox-translations#94 Co-authored-by: Jelmer van der Linde <[email protected]>
@jerinphilip I am re-opening this one just to make sure that we don't forget to add the test cases as well. |
Closing this one as filed #363 for test cases. |
Based on the discussions in mozilla/firefox-translations#51, the translation API will require some changes to be able to specify whether each entry in the batch of text to be translated is HTML or not. This could mean (as @jelmervdl also pointed in mozilla/firefox-translations#51 (comment)) accepting a
ResponseOptions
object per entry of the input batch.Opening this issue so that it can be discussed here and appropriate changes can be done. Taking BlockingService::translateMultiple API as example, the new API could look like:
Please note that this means all ResponseOptions could be different for each entry of
source
and therefore, eachResponse
will have to be constructed accordingly.cc @jerinphilip @kpu @andrenatal
The text was updated successfully, but these errors were encountered: