DeepLcom / deepl-php

Official PHP library for the DeepL language translation API.
MIT License
202 stars 23 forks source link

targetLang="en" is deprecated, please use "en-GB" or "en-US" instead #44

Open kalebheitzman opened 6 months ago

kalebheitzman commented 6 months ago
Execute scheduled task: Autotranslation Fetch (filter_autotranslate\task\autotranslate_task)
... started 22:06:02. Current memory use 29.1 MB.
Debugging increased temporarily due to faildelay of 60
Executing autotranslation fetch jobs...
200 jobs found...
fetching en translation for d41d8cd98f00b204e9800998ecf8427e key...
... used 4 dbqueries
... used 0.015782117843628 seconds
Scheduled task failed: Autotranslation Fetch (filter_autotranslate\task\autotranslate_task),targetLang="en" is deprecated, please use "en-GB" or "en-US" instead.
Backtrace:
* line 156 of /filter/autotranslate/vendor/deeplcom/deepl-php/src/Translator.php: call to DeepL\Translator->buildBodyParams()
* line 108 of /filter/autotranslate/classes/task/autotranslate_task.php: call to DeepL\Translator->translateText()
* line 405 of /lib/classes/cron.php: call to filter_autotranslate\task\autotranslate_task->execute()
* line 208 of /lib/classes/cron.php: call to core\cron::run_inner_scheduled_task()
* line 125 of /lib/classes/cron.php: call to core\cron::run_scheduled_tasks()
* line 186 of /admin/cli/cron.php: call to core\cron::run_main_process()

This isn't technically a bug but it broke our entire app. The Readme might need updated to notify users of this. We have a custom plugin on Moodle that's based entirely on 2 letter locale codes and I haven't gotten into the internals yet to see if it will give us the en-US format. There might be other software out there that suffers from the same problem.

JanEbbing commented 6 months ago

Hi, what is the bug here exactly? The information is in the readme:

Some target languages also include the regional variant according to ISO 3166-1, for example 'en-US', or 'pt-BR'. The source language also accepts null, to enable auto-detection of the source language.

and this is not a recent change, we haven't accepted en as a target language for years.

You can use Translator::getSourceLanguages() and Translator::getTargetLanguages() to query the currently accepted languages from the API, or use the constants we define in the library, note the comments for english:


    /** English language code, may only be used as a source language. */
    public const ENGLISH = 'en';

    /** British English language code, may only be used as a target language. */
    public const ENGLISH_BRITISH = 'en-GB';

    /** American English language code, may only be used as a target language. */
    public const ENGLISH_AMERICAN = 'en-US';
kalebheitzman commented 5 months ago

@JanEbbing that's weird. Our plugin was working fine when using just en as a target language code and it just broke in the last few weeks with that deprecation notice. The documentation states that you have to pass a locale code in order for the glossary to work and unless we change our Moodle install to use regional variants we'll have to hardcode locales into the plugin to make it work with DeepL now.

On the readme it shows $translationResult = $translator->translateText('Hello, world!', 'en', 'fr'); but my understanding is that I now have to use en-US or en-GB, etc and that I cannot use en. Am I misunderstanding something here?

JanEbbing commented 5 months ago

Hi @kalebheitzman - not sure why it broke, but we did not change anything in this regard. DeepL differentiates between a sourceLanguage (language the text is in, can be null in order to be inferred) and a targetLanguage (language to translate the text into) - the targetLanguage needs the regional variant to be specified, else it will throw an error (unfortunately only once you actually call the function, as we want users of older library versions to be able to use languages released after the library version).

In the example

$translationResult = $translator->translateText('Hello, world!', 'en', 'fr');

we call translate with a sourceLanguage of 'en' and a targetLanguage of 'fr', which is fine. On the other hand,

$translationResult = $translator->translateText('Bonjour, le monde!', 'fr', 'en');

would throw an error, you would need to write something like

$translationResult = $translator->translateText('Bonjour, le monde!', 'fr', 'en-GB');

nlemoine commented 3 months ago

@JanEbbing I stumbled on that issue too in an app that basically provide a source and target langauge selectors the user can select from.

The list of those selectors are hydrated from DeepL /languages endpoint. What's confusing here is that we expect (as developers) those lists items to be valid when using them and not waiting for a translation process to run for throwing errors. Sadly, there's no way to identify the deprecated codes (e.g. en and pt) and I have to keep a target language blacklist that I would prefer (I think we all do) be maintained from your side.

One way to solve this would be to introduce a is_deprecated (or whatever name) field on the returned languages by your API (just like supports_formality) which could be then mapped to a prop on the DeepL\Language class. That would solve developer's problems with this.

What do you think?

JanEbbing commented 2 months ago

Hi @nlemoine , please note that the /languages endpoint has a type parameter that can be passed to specify if you want to retrieve source or target languages, which defaults to source, hence you would get en and pt, as they are valid source languages. If I set type to target, I correctly get the regional variants (EN-US, PT-BR, etc). Does this solve your issue around a blacklist?

E: I guess the issue is that it is not mentioned on the dedicated docs page about languages, but only in the OpenAPI spec here