Opened 4 years ago

Closed 3 years ago

#2912 closed defect (fixed)

raster_what() not compliant with localised version of GRASS

Reported by: andbal Owned by: grass-dev@…
Priority: major Milestone: 7.0.5
Component: Python Version: svn-trunk
Keywords: grass.script python translation Cc:
CPU: x86-64 Platform: Linux

Description

Writing a python script, I found that the grass.script.raster_what() output dictionary has different keys depending on language used in GRASS. The following python code:

point = grass.raster_what('clump', [[x,y]])[0]
print(type(point), point)

returns (with Italian GRASS):

<type 'dict'> {'clump': {'color': '143:247:067', 'etichetta': '', 'valore': '1'}}

Changing language to English, returns:

<type 'dict'> {'clump': {'color': '151:149:192', 'value': '1', 'label': ''}}

Changing language to German, returns:

<type 'dict'> {'clump': {'Farbe': '099:113:108', 'value': '1', 'label': ''}}

The translation seems to depend on the GRASS localisations and make impossible access to the dictionary keys in a general way.

Anyway a workaround could be achieved using grass.script.read_command('r.what') and accessing to the list values.

Attachments (1)

raster_what.diff (3.1 KB) - added by lucadelu 4 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 in reply to:  description ; Changed 4 years ago by mlennert

Replying to andbal:

Writing a python script, I found that the grass.script.raster_what() output dictionary has different keys depending on language used in GRASS.

This comes from line 191 in lib/python/script/raster.py where these labels are noted as translatable strings. I agree that this is a very bad idea and that such dictionary keys should not be translatable.

Unless anyone gives a good reason for the contrary, I will change the vector of labels to non-translatable. This might break existing scripts, but I think this is necessary.

comment:2 in reply to:  1 Changed 4 years ago by marisn

Replying to mlennert:

Replying to andbal:

Writing a python script, I found that the grass.script.raster_what() output dictionary has different keys depending on language used in GRASS.

This comes from line 191 in lib/python/script/raster.py where these labels are noted as translatable strings. I agree that this is a very bad idea and that such dictionary keys should not be translatable.

Unless anyone gives a good reason for the contrary, I will change the vector of labels to non-translatable. This might break existing scripts, but I think this is necessary.

Then also add line 184 ("No data").

This also needs to be explicitly stated in docstring that it is callers responsibility to use localized version of key values before they are displayed to the end user.

comment:3 in reply to:  1 ; Changed 4 years ago by lucadelu

Replying to mlennert:

Replying to andbal:

Writing a python script, I found that the grass.script.raster_what() output dictionary has different keys depending on language used in GRASS.

This comes from line 191 in lib/python/script/raster.py where these labels are noted as translatable strings. I agree that this is a very bad idea and that such dictionary keys should not be translatable.

Unless anyone gives a good reason for the contrary, I will change the vector of labels to non-translatable. This might break existing scripts, but I think this is necessary.

In the attached diff I propose a change, because the actual behavior is useful for GUI modules but not for other uses

Changed 4 years ago by lucadelu

Attachment: raster_what.diff added

comment:4 in reply to:  3 ; Changed 4 years ago by annakrat

Replying to lucadelu:

In the attached diff I propose a change, because the actual behavior is useful for GUI modules but not for other uses

I would prefer to avoid the gui parameter. Instead we could have a wrapper function raster_what in wxpython/core/utils.py which would replace the labels with the translatable ones? Sorry, no time for this right now, maybe later.

comment:5 in reply to:  4 ; Changed 4 years ago by lucadelu

Replying to annakrat:

I would prefer to avoid the gui parameter. Instead we could have a wrapper function raster_what in wxpython/core/utils.py which would replace the labels with the translatable ones? Sorry, no time for this right now, maybe later.

Can you explain a little bit more why you don't like gui parameter? Your proposal seems to me a duplication of code...

comment:6 in reply to:  5 ; Changed 4 years ago by annakrat

Replying to lucadelu:

Replying to annakrat:

I would prefer to avoid the gui parameter. Instead we could have a wrapper function raster_what in wxpython/core/utils.py which would replace the labels with the translatable ones? Sorry, no time for this right now, maybe later.

Can you explain a little bit more why you don't like gui parameter? Your proposal seems to me a duplication of code...

I didn't like the gui parameter, perhaps localized=False would make more sense when you want to use localized names in a script, not just in the gui.

comment:7 in reply to:  6 ; Changed 4 years ago by lucadelu

Replying to annakrat:

I didn't like the gui parameter, perhaps localized=False would make more sense when you want to use localized names in a script, not just in the gui.

Ok, if it is only parameter's name, it is not problem to change. I like localized. Could I submit the code with localized parameter instead gui one?

comment:8 in reply to:  7 ; Changed 4 years ago by annakrat

Replying to lucadelu:

Replying to annakrat:

I didn't like the gui parameter, perhaps localized=False would make more sense when you want to use localized names in a script, not just in the gui.

Ok, if it is only parameter's name, it is not problem to change. I like localized. Could I submit the code with localized parameter instead gui one?

Sure, if you tested it with different locale and it behaves as expected.

comment:9 in reply to:  8 ; Changed 4 years ago by lucadelu

Replying to annakrat:

Sure, if you tested it with different locale and it behaves as expected.

committed with test in r67874. Tests are needed before backport.

comment:10 in reply to:  9 Changed 4 years ago by andbal

Replying to lucadelu:

Tests are needed before backport.

I tried the testsuite code with two languages other than english and gave back 'OK' in both cases.

EDIT: No more tester but seems to works anyway, I close the ticket. Thank you.

Last edited 4 years ago by andbal (previous) (diff)

comment:11 Changed 4 years ago by andbal

Resolution: fixed
Status: newclosed

comment:12 Changed 4 years ago by annakrat

Resolution: fixed
Status: closedreopened

As far as I know, it hasn't been backported yet.

comment:13 Changed 4 years ago by martinl

Milestone: 7.0.47.0.5

comment:14 Changed 3 years ago by martinl

Can be backport done?

comment:15 in reply to:  14 Changed 3 years ago by annakrat

Replying to martinl:

Can be backport done?

Yes, please do it.

comment:16 Changed 3 years ago by lucadelu

Resolution: fixed
Status: reopenedclosed

In 69242:

python script: backported raster_what function to work with or without localized labels fixed #2912

Note: See TracTickets for help on using tickets.