search

argosopentech / argos-translate

Open-source offline translation library written in Python
https://www.argosopentech.com
MIT License
3.77k stars 277 forks source link

Use the new TranslationResult properties from ctranslate2 #404

Closed pirhoo closed 5 months ago

pirhoo commented 6 months ago

Hello,

I noticed a deprecation warning when running the FewShotTranslation.apply_packaged_translation method:

DeprecationWarning: Reading the TranslationResult object as a list of dictionaries is deprecated and will be removed in a futur version. Please use the object attributes as described in the documentation: https://opennmt.net/CTranslate2/python/ctranslate2.TranslationResult.html

This could lead to an exception:

> translated_tokens += translated_batch[i]["tokens"]                                                             
SystemError: <built-in method __getitem__ of PyCapsule object at 0x7d8642cd7d80> returned a result with an exception set

So I simply used the new properties as suggested by ctranslate2.

Let me know if you want me to add a safer access to the properties for retro-compatibility.

Thanks for the great tool!

pirhoo commented 5 months ago

Hello :wave:

Just wanted to check if you could review my PR ; Unfortunately this error is a bit blocking for me.

argosopentech commented 5 months ago

Good catch thank you!

argosopentech commented 5 months ago

I pushed this change in v1.9.5

pirhoo commented 5 months ago

Wonderful, thanks!

PJ-Finlay commented 5 months ago

Why was this causing issues for you? This fix is definitely correct but I've never seen the deprecated version cause problems.

pirhoo commented 5 months ago

The error started to appear for me when upgraded Argos Translate from 1.7 to 1.9: https://github.com/ICIJ/es-translator/commit/a2e329b238e200978922e018ce385002d7fceb44

As you can see, CTranslate2 is a transitive dependency for me. No idea why the error did not appear sooner (I saw you upgraded it prior to Argos Translate 1.8), really weird!