Deprecate unnecessary WebAssembly functions in Response

[jerinphilip]

The C++ API uses Response as data. source and target are symmetric in structure and this has benefits.

Python bindings almost entirely rely on pybind11 metaprogramming and composition to obtain an easy to maintain, binding friendly value-type.

bergamot-translator/bindings/python/bergamot.cpp

Lines 166 to 170 in a94725b

	py::class_<Response>(m, "Response")
	.def(py::init<>())
	.def_readonly("source", &Response::source)
	.def_readonly("target", &Response::target)
	.def_readonly("alignments", &Response::alignments);

It is also easier from a maintenance and development perspective that the accessors be consistent across bindings exported.

With a JSON counterpart also coming for Native Messaging (XapaJIaMnu/translateLocally#93), it is important to make the access consistent across Python, C++ and JavaScript, which the developers use.

WebAssembly is also, just bindings. However, WebAssembly appears to planted methods some of which are dead code as of now, creating another means to access the same thing.

bergamot-translator/src/translator/response.h

Lines 65 to 75 in a94725b

	/// @param [in] sentenceIdx: The index representing the sentence where 0 <= sentenceIdx < Response::size()
	ByteRange getSourceSentenceAsByteRange(size_t sentenceIdx) const { return source.sentenceAsByteRange(sentenceIdx); }
	/// Returns the translated sentence (in terms of byte range) corresponding to sentenceIdx.
	///
	/// @param [in] sentenceIdx: The index representing the sentence where 0 <= sentenceIdx < Response::size()
	ByteRange getTargetSentenceAsByteRange(size_t sentenceIdx) const { return target.sentenceAsByteRange(sentenceIdx); }
	const std::string &getOriginalText() const { return source.text; }
	const std::string &getTranslatedText() const { return target.text; }

bergamot-translator/wasm/bindings/response_bindings.cpp

Lines 25 to 33 in a94725b

	EMSCRIPTEN_BINDINGS(response) {
	class_<Response>("Response")
	.constructor<>()
	.function("size", &Response::size)
	.function("getQualityScores", &getQualityScores)
	.function("getOriginalText", &Response::getOriginalText)
	.function("getTranslatedText", &Response::getTranslatedText)
	.function("getSourceSentence", &Response::getSourceSentenceAsByteRange)
	.function("getTranslatedSentence", &Response::getTargetSentenceAsByteRange);

This issue proposes to deprecate the WebAssembly added extra functions and constructing bindings via composition to make access to source/target identical and consistent with usages elsewhere.

The changes proposed in this issue will break the WebAssembly test page, but can be fixed quite easily. Also some stuff at https://github.com/mozilla/firefox-translations/blob/6892ad64164787adf1e284141cbd0d10e71fb40d/extension/controller/translation/translationWorker.js at some point, but assuming worker draws from test-page it's solved already by fixing the test-page. It is easier to incur the cost now than later and have all exports of at least Response evolve together.

I am not aware of performance hits we will incur with the convenience mentioned above, but a consistent API expecting the platform (emscripten/WebAssembly) to fix the performance hits in future is what I propose we should still go for.

[jerinphilip]

Replacing .function with .property providing only a getter (no setter) is a potential way to go - https://emscripten.org/docs/api_reference/bind.h.html#_CPPv4NK6class_8propertyEPKcM9ClassType9FieldType.

Experimenting with this for alignments at jerinphilip@60e88b6.