Error in chriskonnertz/deeply/src/ChrisKonnertz/DeepLy/ResponseBag/TranslationBag.php
domih opened this issue · 18 comments
Note: not urgent, just trying it out.
$text = <<<EOS
The Center for American Progress puts it simply: “Americans work longer hours than workers in most other developed countries, including Japan, where there is a word, ‘karoshi’, for ‘death by overwork.’ The typical American middle-income family puts in an average of 11 more hours a week in 2006 than it did in 1979.” The end result is that we all live in a structurally toxic work world to use the title of an article written by Anne-Marie Slaughter, the author of “Unfinished Business: Women Men Work Family” published in 2015. The situation is bad for men and even worse for women. So what can we as individuals do about it without jeopardizing our job and reputation? Scott McDowell offers a useful starting point in a FastCompany article: Steps We Can All Take To Defy Our Culture Of Overwork:
If you are a boss, reduce your team’s hours.
Take your vacations.
Schedule free time.
Let your mind wander.
Stop talking how busy you are.
“In the past labor unions fought for worker sanity,” Scott McDowell adds. “In our current work culture we have to take individual responsibility for our well-being and for that of those around us. It’s time to alter the norms from within; our economy, health, productivity, and happiness depend on it.”
EOS;
$translatedText = $deepLy->translate($text, DeepLy::LANG_EN, DeepLy::LANG_FR);
Returns:
( ! ) Notice: Undefined offset: 0 in /var/www/html/libs/vendor/chriskonnertz/deeply/src/ChrisKonnertz/DeepLy/ResponseBag/TranslationBag.php on line 81
Call Stack
Time Memory Function Location
1 0.0006 367048 {main}( ) .../tryme.php:0
2 0.0103 1837440 ChrisKonnertz\DeepLy\DeepLy->translate( ) .../tryme.php:35
3 3.2932 1864856 ChrisKonnertz\DeepLy\ResponseBag\TranslationBag->getTranslation( ) .../DeepLy.php:239
( ! ) Notice: Trying to get property of non-object in /var/www/html/libs/vendor/chriskonnertz/deeply/src/ChrisKonnertz/DeepLy/ResponseBag/TranslationBag.php on line 87
Call Stack
Time Memory Function Location
1 0.0006 367048 {main}( ) .../tryme.php:0
2 0.0103 1837440 ChrisKonnertz\DeepLy\DeepLy->translate( ) .../tryme.php:35
3 3.2932 1864856 ChrisKonnertz\DeepLy\ResponseBag\TranslationBag->getTranslation( ) .../DeepLy.php:239
Le Center for American Progress le dit simplement:"Les Américains travaillent plus d'heures que les travailleurs dans la plupart des autres pays développés, y compris au Japon, où il y a un mot," karoshi ", pour" mort par surmenage "; la famille américaine typique à revenu moyen consacre en moyenne 11 heures de plus par semaine en 2006 qu'en 1979;" Le résultat final est que nous vivons tous dans un monde du travail structurellement toxique pour utiliser le titre d'un article écrit par Anne-Marie Sla ". La situation est mauvaise pour les hommes et pire encore pour les femmes. Que pouvons-nous donc faire, en tant qu'individus, pour y remédier sans mettre en péril notre travail et notre réputation? Prenez vos vacances. Prévoir du temps libre. Laisse ton esprit vagabonder. Arrête de dire à quel point tu es occupé. Dans le passé, les syndicats se sont battus pour la santé mentale des travailleurs ", ajoute Scott McDowell. Dans notre culture de travail actuelle, nous devons assumer la responsabilité individuelle de notre bien-être et de celui de ceux qui nous entourent. Il est temps de changer les normes de l'intérieur; notre économie, notre santé, notre productivité et notre bonheur en dépendent."
Hello,
thank you for creating this issue. I will take a look at this!
Which version did you use? Please take a look at this constant: ChrisKonnertz\DeepLy\DeepLy::version
On the master branch it works fine:
<?php
use ChrisKonnertz\DeepLy\DeepLy;
/**
* Minimal class autoloader
*
* @param string $class Full qualified name of the class
*/
function miniAutoloader($class)
{
require __DIR__ . '/../src/' . $class . '.php';
}
spl_autoload_register('miniAutoloader');
$deepLy = new ChrisKonnertz\DeepLy\DeepLy();
$text = <<<EOS
The Center for American Progress puts it simply: “Americans work longer hours than workers in most other developed countries, including Japan, where there is a word, ‘karoshi’, for ‘death by overwork.’ The typical American middle-income family puts in an average of 11 more hours a week in 2006 than it did in 1979.” The end result is that we all live in a structurally toxic work world to use the title of an article written by Anne-Marie Slaughter, the author of “Unfinished Business: Women Men Work Family” published in 2015. The situation is bad for men and even worse for women. So what can we as individuals do about it without jeopardizing our job and reputation? Scott McDowell offers a useful starting point in a FastCompany article: Steps We Can All Take To Defy Our Culture Of Overwork:
If you are a boss, reduce your team’s hours.
Take your vacations.
Schedule free time.
Let your mind wander.
Stop talking how busy you are.
“In the past labor unions fought for worker sanity,” Scott McDowell adds. “In our current work culture we have to take individual responsibility for our well-being and for that of those around us. It’s time to alter the norms from within; our economy, health, productivity, and happiness depend on it.”
EOS;
$translatedText = $deepLy->translate($text, DeepLy::LANG_EN, DeepLy::LANG_FR);
echo '<pre>'.$translatedText.'</pre>';Printed:
Le Center for American Progress le dit simplement:"Les Américains travaillent plus d'heures que les travailleurs dans la plupart des autres pays développés, y compris au Japon, où il y a un mot," karoshi ", pour" mort par surmenage "; la famille américaine typique à revenu moyen consacre en moyenne 11 heures de plus par semaine en 2006 qu'en 1979;" Le résultat final est que nous vivons tous dans un monde du travail structurellement toxique pour utiliser le titre d'un article écrit par Anne-Marie Sla ". La situation est mauvaise pour les hommes et pire encore pour les femmes. Que pouvons-nous donc faire, en tant qu'individus, pour y remédier sans mettre en péril notre travail et notre réputation? Scott McDowell offre un point de départ utile dans un article de FastCompany: Steps We Can All Take To Defy Our Culture Of Overwork:" La culture du surmenage: Si vous êtes un patron, réduisez les heures de travail de votre équipe. Prenez vos vacances. Prévoir du temps libre. Laisse ton esprit vagabonder. Arrête de dire à quel point tu es occupé. Dans le passé, les syndicats se sont battus pour la santé mentale des travailleurs ", ajoute Scott McDowell. Dans notre culture de travail actuelle, nous devons assumer la responsabilité individuelle de notre bien-être et de celui de ceux qui nous entourent. Il est temps de changer les normes de l'intérieur; notre économie, notre santé, notre productivité et notre bonheur en dépendent."
(PHP 7.0.13, Apache, Windows, error_reporting E_ALL)
However, I have added some lines of code that should catch the problem before a notice is created and throw a nice exception.
const VERSION = '1.3.0';
...$ composer require chriskonnertz/deeply
Using version ^1.3 for chriskonnertz/deeply
./composer.json has been updated
Loading composer repositories with package information
Updating dependencies (including require-dev)
Package operations: 1 install, 0 updates, 0 removals
- Installing chriskonnertz/deeply (v1.3.0): Downloading (100%)
By the way, I'm using PHP 7.1.9.
Strange. The Travis-CI tests cover PHP 7.1: https://github.com/chriskonnertz/DeepLy/blob/master/.travis.yml#L6
Can you please update again (to version 1.3.1), then run the code and tell me if you still get the same exception or if it is a BagException?
1.3.1 outputs:
DeepLy API call resulted in a malformed result - beams array is empty in translation 3
Note: the translation goes through OK when directly using https://www.deepl.com/translator
I added some logging to CurlHttpClient::callApi(...):
public function callApi($url, array $payload, $method)
{
\Util::logDebug("CurlHttpClient::callApi($url, ..., $method)");
//...
\Util::logDump("Deeply / \$jsonData\n\n", $jsonData);
\Util::logDump("Deeply / \$jsonData pretty\n\n", \Util::jsonPrettyPrint($jsonData));
\Util::logDump("Deeply/ \$jsonData as array\n\n", json_decode($jsonData, true));
$rawResponseData = curl_exec($curl);
\Util::logDump("Deeply / \$rawResponseData\n\n", $rawResponseData);
\Util::logDump("Deeply / \$rawResponseData pretty\n\n", \Util::jsonPrettyPrint($rawResponseData));
\Util::logDump("Deeply / \$rawResponseData as array\n\n", json_decode($rawResponseData, true));
//...
}
See attached resulting log.
wr-2017-09-19-18.log.txt
If, you comment out this:
// if (sizeof($translation->beams) == 0) {
// throw new BagException(
// 'DeepLy API call resulted in a malformed result - beams array is empty in translation '.$index
// );
// }
And add further down the test on the array size:
foreach ($this->responseContent->translations as $translation) {
// The beams array contains 1-n translation alternatives.
// The first one (index 0) is the "best" one (best score)
if(count($translation->beams)){
$beam = $translation->beams[0];
if ($translatedText !== '') {
$translatedText .= ' ';
}
$translatedText .= $beam->postprocessed_sentence;
}
}
It goes through. So the question you should ask yourself: is an empty beams array an error?
Finally, I have these questions:
- Why do you need to call the API twice?
- When using https://www.deepl.com/translator the carriage returns are maintained. When using Deeply, there are eliminated. Is there a way to maintain them?
TIA!
It goes through. So the question you should ask yourself: is an empty beams array an error?
"It goes through" = So you get the translation then?
"is an empty beams array an error" I don't know, welcome to the "let's guess how the API works" game. ;-) I guess it is an error but this is just a guess. My big problem is that I am not able to reproduce the behaviour that you have experienced. So all I do is guessing / assuming what might cause the issue.
Why do you need to call the API twice?
The reason is a limitation of the API. It is not able to translate a text (=one string) that consists of multiple sentences. However it can translate multiple sentences (=array of strings) in one API call. And it is able to split a text into sentences. So the current implementation first makes a call that splits texts into sentences and then makes antoher call that translates these sentences.
Since the API is not documented I do not know if it is possible to merge both calls into one "split and translate" call - which would be great.
If you only want to translate a single sentence, you can call the translate($text, $to = self::LANG_EN, $from = self::LANG_AUTO, $joinSentences = false) method with joinSentences set to true.
If you have an array of sentences there will be a method that accepts an array as parameter and directly translates the sentences (=array items) without an additional API call. However, that has not been implemented yet.
When using https://www.deepl.com/translator the carriage returns are maintained. When using Deeply, there are eliminated. Is there a way to maintain them
This happens when the text is split into sentences. DeepLy sends something like "Hello world!\r\nHow are you?" (string) to the API and the response is ["Hallo Welt!", "Wie geht es dir?"] (array)". So this happens outside of DeepLy. When joining the sentences to a single string DeepLy could at line break but between all sentences but that would be pure guessing. So I guess the best way would be to split the text via $parts = explode('\r\n', $text), then let the API split all $parts and translate them and then put them together via $text = implode(PHP_EOL, $translatedParts).
Made a commit: ada88e9
The translateSentences() method now expects an array of strings (sentences) as the first parameter and internally only makes one API call.
"It goes through" = So you get the translation then?
Yes.
I'll close this issue. I cannot reproduce the error so I cannot really fix it. I made two commits that may help a little bit and I added the idea of keeping line breaks to the to-do-list.