Import translation function instead of using a buildin

[grass-svn2git]

Reported by @wenzeslaus on 17 Sep 2014 01:35 UTC ''Here are my notes about this issue since I was forgetting about it.''

On some occasions, some of GRASS functions does not work because of translation function (_ - underscore). The problem occurs on some strange conditions and when you are using Python https://docs.python.org/2/library/doctest.html doctest which we are using more and more.

There is already several workarounds in wxGUI and also testing framework as [https://trac.osgeo.org/grass/wiki/GSoC/2014/TestingFrameworkForGRASS#Findingandrunningthetestmodules described here]:

doctests currently don't work with grass.script unless you call a [https://trac.osgeo.org/grass/browser/grass/trunk/gui/wxpython/core/toolboxes.py?rev=60218#L630 set of functions] to deal with function (underscore) because of installing translate function as buildin function while _ function is used also in doctest.

Grep for function do_doctest_gettext_workaround to see definition and how it is used.

I already did this change for wxGUI more then a year ago and it seems to work. It was necessary because there was some problem with not translated files and this was only way to fix it besides going against documentation.

For now I don't know how lib/python, scripts, and temporal differs in translations, so this should be clarified before changing it.

See comment:5:ticket:1739 for deep discussion of the topic.

For wxGUI this was implemented in the https://trac.osgeo.org/grass/changeset/57219 and https://trac.osgeo.org/grass/changeset/57220 as a result of https://trac.osgeo.org/grass/ticket/1739 and other investigation.

This might be a blocker for some solutions of https://trac.osgeo.org/grass/ticket/2142. The new method should be flexible allowing to obtain translations from two sources simultaneously.

Basically, it is necessary to remove all occurrences of:

gettext.install('grasswxpy', os.path.join(os.getenv("GISBASE"), 'locale'), unicode = True)

By one code based on this line in some module:

_ = gettext.translation('grasswxpy', os.path.join(os.getenv("GISBASE"), 'locale')).ugettext 

The actual implementation must be more complicated (changeset:57220#file0).

Then each file, depending on the name and placement of translation function, must import:

from grass.script.utils import _
from grass.script.translations import _
from grass.*.utils import _
from grass.*.translations import _
from grass.translations import _

Probably grass.translations is the best since in GUI a module inside some other package is creating dependencies (changeset:57219#file10, changeset:57219#file13).

GRASS GIS version and provenance

svn-releasebranch70

Migrated-From: https://trac.osgeo.org/grass/ticket/2425

[grass-svn2git]

Comment by @glynnc on 17 Sep 2014 21:20 UTC Replying to [ticket:2425 wenzeslaus]:

Basically, it is necessary to remove all occurrences of:

gettext.install('grasswxpy', os.path.join(os.getenv("GISBASE"), 'locale'), unicode = True)

By one code based on this line in some module:

_ = gettext.translation('grasswxpy', os.path.join(os.getenv("GISBASE"), 'locale')).ugettext 

I'd suggest that what's actually "necessary" is to fix whatever can't cope with "_" being a built-in function.

It shouldn't be necessary to explicitly import "_" into each and every file. This is why Python has a (mutable) built-in namespace.

[grass-svn2git]

Comment by @wenzeslaus on 17 Sep 2014 22:02 UTC Replying to [comment:1 glynn]:

Replying to [ticket:2425 wenzeslaus]:

Basically, it is necessary to remove all occurrences of:

gettext.install('grasswxpy', os.path.join(os.getenv("GISBASE"), 'locale'), unicode = True)

By one code based on this line in some module:

_ = gettext.translation('grasswxpy', os.path.join(os.getenv("GISBASE"), 'locale')).ugettext 

I'd suggest that what's actually "necessary" is to fix whatever can't cope with "_" being a built-in function.

That's a good point but both doctest and gettext are part of Python and they are still not compatible. I would say that they don't want to change it. The issue is known for a long time and by a lot of people:

• https://mail.python.org/pipermail/python-list/2004-February/248035.html (http://bytes.com/topic/python/answers/28870-problem-mixing-doctest-gettext-_ other part of discussion
• http://sheep.art.pl/Gettext%20and%20Doctest%20Together%20in%20Python
• https://github.com/nose-devs/nose/issues/393
• (however, no http://bugs.python.org/issue?@filter=status&@filter=components&components=4&status=1&@columns=id,activity,title,status&@sort=-activity Python issue found)

A lot of people/projects went the way of using explicit import (rather then (implicit) buildin), most notably Django (although I'm aware of some recent strange Django decisions):

• https://docs.djangoproject.com/en/dev/topics/i18n/translation/

It shouldn't be necessary to explicitly import "_" into each and every file. This is why Python has a (mutable) built-in namespace.

Yes, that's true, Python allows that but I don't think that it is a good practice. Of course, gettext is a bit special but this does not mean that it will save it from the risks of using buildin (doctest is probably using the buildin for the very same reason, it considers itself special).

The other motivation is that wxGUI needed some change (since the former state was not working) and the only two solutions we found were putting install call to each file (which is agiast buildin and gettext documentation) or using explicit imports.

I think we should you the same practice everywhere but what remained me about this issue was that I have seen in some Python editor an error message which was saying that _(...) does not take any arguments (or something like that, I was not able to get or reproduce the message). This is the kind of message is the same as given by doctest, so I guess explicit imports would solve both issues.

The number of imports is high for most of the files (with the exception of simple scripts), so I don't consider this as a huge issue.

We should decide this some day. I wouldnt do the change for 7.0, but for trunk/7.1 it should be safe.

[grass-svn2git]

Comment by @glynnc on 17 Sep 2014 23:34 UTC Replying to [comment:2 wenzeslaus]:

That's a good point but both doctest and gettext are part of Python and they are still not compatible. I would say that they don't want to change it. The issue is known for a long time and by a lot of people:

In which case, I would say that doctest loses. Either come up with a workaround for doctest, or live without it. It isn't acceptable to require every programmer to jump through hoops for the sake of doctest.

The other motivation is that wxGUI needed some change (since the former state was not working) and the only two solutions we found were putting install call to each file (which is agiast buildin and gettext documentation) or using explicit imports.

I believe that wxPython has its own "_" related to its own I18N subsystem. If possible, I would prefer to "fix" wxPython. And if that isn't possible, I would prefer that any sub-optimal solutions don't end up infecting non-wxGUI code.

I think we should you the same practice everywhere but what remained me about this issue was that I have seen in some Python editor an error message which was saying that _(...) does not take any arguments (or something like that, I was not able to get or reproduce the message). This is the kind of message is the same as given by doctest, so I guess explicit imports would solve both issues.

This sounds like the interactive shell using "_" as a variable to hold the result of the last expression:

> 2 + 3
5
> _
5
> __builtins__._
5

It may be that doctest is also using "_" in this way in order to faithfully mimic the behaviour of the interactive shell. Actually, there's no "may" about it; the behaviour is implemented by https://docs.python.org/2.7/library/sys.html#sys.displayhook sys.displayhook:

sys.displayhook(value)

    If value is not None, this function prints it to sys.stdout, and saves it in __builtin__._.

    sys.displayhook is called on the result of evaluating an expression entered in an interactive Python session. The display of these values can be customized by assigning another one-argument function to sys.displayhook.

doctest.py explicitly restores this function to its initial value (from https://docs.python.org/2.7/library/sys.html#sys.__displayhook__ sys.displayhook) while executing the tests:

        # Make sure sys.displayhook just prints the value to stdout
        save_displayhook = sys.displayhook
        sys.displayhook = sys.__displayhook__

Indeed, trying to debug code which uses () by pasting it into an interactive session often fails because the shell assigns `builtins.` every time you enter a statement. But that's just how it is; the function has to be called _() so that xgettext can identify strings for translation.

One possibility is to replace both sys.displayhook and sys.__displayhook__ with something which preserves __builtin__._ while running doctest. Obviously, any examples which rely upon the interpreter setting "" won't work, but that's easy enough to avoid (and in any case, they won't work if the code use "from whatever import ", because variables in the current module override built-ins).

[grass-svn2git]

Comment by @wenzeslaus on 2 Nov 2014 00:03 UTC I still think that not using _ as a buildin for translations is a good idea adopted by other projects such as Django (comment:5:ticket:1739).

If user is trying something in Python command line (in system terminal, in wxGUI or elsewhere), he or she will get strange errors in case the Python functions will print some warnings, errors or any translatable text. And this is wrong I would say.

>>> from grass.script.core import run_command
>>> run_command('g.region', rast_='elevation')
0
>>> run_command('g.region', _rast='elevation')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File ".../etc/python/grass/script/core.py", line 355, in run_command
    ps = start_command(*args, **kwargs)
  File ".../etc/python/grass/script/core.py", line 334, in start_command
    args = make_command(prog, flags, overwrite, quiet, verbose, **options)
  File ".../etc/python/grass/script/core.py", line 283, in make_command
    warning(_("To run the module add underscore at the end"
TypeError: 'int' object is not callable
>>> _
0

It will work in IPython, however:

In [1]: from grass.script.core import run_command

In [2]: run_command('g.region', rast_='elevation')
Out[2]: 0

In [3]: run_command('g.region', _rast='elevation')
WARNING: To run the module add underscore at the end of the option <rast>
         to avoid conflict with Python keywords. Underscore at the
         beginning is depreciated in GRASS GIS 7.0 and will be removed in
         version 7.1.
Out[3]: 0

In [4]: _
Out[4]: <bound method NullTranslations.gettext of <gettext.NullTranslations instance at 0x7ff801cc45a8>>

Anyway, using explicit import of _ rather then "install" to buildins would solve not only doctest and Python interactive interpreter but would also satisfy number of code checkers which complain about undefined _ symbol. Moreover, "explicit is better than implicit" is from Zen of Python (http://legacy.python.org/dev/peps/pep-0020/ PEP20).

[grass-svn2git]

Comment by @neteler on 5 May 2016 14:08 UTC Milestone renamed

[grass-svn2git]

Comment by @wenzeslaus on 14 Nov 2016 02:25 UTC The change is substantial, so it can't make it to 7.2 at this point.

[grass-svn2git]

Comment by @neteler on 26 Jan 2018 11:40 UTC Ticket retargeted after milestone closed

[grass-svn2git]

Comment by @landam on 28 Jan 2018 10:56 UTC What is status of this issue?

[grass-svn2git]

Comment by @wenzeslaus on 9 Jun 2018 22:44 UTC This is potentially relevant for the Python 3 GSoC 2018 project.

Replying to [comment:8 martinl]:

What is status of this issue?

Not implemented. This already works in wxGUI successfully for some time. All points from the discussion are likely still valid.