Underscore to avoid Python keywords used improperly in grass.script

[wenzeslaus]

When a module parameter is the same as one of Python keywords, we are adding and underscore before the name of parameter, so call of a module:

run_command('s.module', lambda="abc")

becomes

run_command('s.module', _lambda="abc")

But this is wrong, according to ​PEP8 and commonly used standard, prefixed underscore means private. To avoid conflicts you should use underscore at the end:

_single_leading_underscore: weak "internal use" indicator. E.g. from M import * does not import objects whose name starts with an underscore.

single_trailing_underscore_: used by convention to avoid conflicts with Python keyword, e.g.

Tkinter.Toplevel(master, class_='ClassName')

If there are no objections I will commit the change soon hopefully with tests and documentation (in case it is not already documented). Please object now.

I plan to commit in backwards compatible manner, so the old syntax will work (for some time).

The code which will change is in grass.script.core and will look like this:

            if opt.startswith('_'):
                opt = opt[1:]
            elif opt.endswith('_'):
                opt = opt[:-1]

or this:

            if opt.endswith('_'):
                opt = opt[:-1]

This should go to 7.0 to allow usage of the good way.

[glynn]

Replying to wenzeslaus:

The code which will change is in grass.script.core and will look like this:

            if opt.startswith('_'):
                opt = opt[1:]
            elif opt.endswith('_'):
                opt = opt[:-1]

No objections, although opt = opt.strip('_') would probably suffice. GRASS option names will never start or end with an underscore.

[wenzeslaus]

Replying to glynn:

No objections, although opt = opt.strip('_') would probably suffice. GRASS option names will never start or end with an underscore.

I will strip just one at the begging or one at the end. I don't want people to use __lambda__ or something like this.

I even consider to write to doc that _lambda is obsolete (in 7.0) and in next versions (7.1 or 7.5) I would include a warning. Finally, I would remove (7.2 or 8.0) the condition completely. This would allow for GRASS option names starting with underscore which might be used for some internal purpose (although this is probably not a good idea).

[zarch]

Replying to wenzeslaus:

Replying to glynn:

No objections, although opt = opt.strip('_') would probably suffice. GRASS option names will never start or end with an underscore.

I will strip just one at the begging or one at the end. I don't want people to use __lambda__ or something like this.

+1

I even consider to write to doc that _lambda is obsolete (in 7.0) and in next versions (7.1 or 7.5) I would include a warning. Finally, I would remove (7.2 or 8.0) the condition completely. This would allow for GRASS option names starting with underscore which might be used for some internal purpose (although this is probably not a good idea).

Why not include the warning already in grass 7.0 and then clean the code directly in grass 7.1? Consider our long release time 7.2 means 6/8 years from now... and it seem too long to me. :-)

[wenzeslaus]

Replying to zarch:

Replying to wenzeslaus:

I even consider to write to doc that _lambda is obsolete (in 7.0) and in next versions (7.1 or 7.5) I would include a warning. Finally, I would remove (7.2 or 8.0) the condition completely. This would allow for GRASS option names starting with underscore which might be used for some internal purpose (although this is probably not a good idea).

Why not include the warning already in grass 7.0 and then clean the code directly in grass 7.1? Consider our long release time 7.2 means 6/8 years from now... and it seem too long to me. :-)

I think that 6 years is average for major version not minor :-) but yes, I committed the underscore suffix in r62196 with the warning for prefixed underscore. This should be backported as is to 70 while in trunk we should remove the whole prefix part. I plan to do it in few days.

[neteler]

What is the state here?

[wenzeslaus]

r62196 was backported in r62256. The differences now are only in unicode handling (related to Python 3 I suppose). I think we can leave the warning in trunk for some time. Closing the ticket. Right before 7.1 we can replace the warning by an error. Later we can remove all the code for underscore prefix.