Opened 11 years ago

Last modified 8 years ago

#1832 new defect

Python scripts are with mixed tabs and spaces

Reported by: klo Owned by: grass-dev@…
Priority: normal Milestone: 6.4.6
Component: Python Version: unspecified
Keywords: indent Cc:
CPU: Unspecified Platform: Unspecified

Description

I noticed that majority of python files in GRASS distribution are with mixed tabs and spaces. Mixing tabs and spaces is not good idea: http://www.python.org/dev/peps/pep-0008/#tabs-or-spaces

In OSGeo4W, python files with mixed tabs and spaces can be found in these folders and/or their sub-folders:

C:\OSGeo4W\apps\grass\grass-6.4.3RC1\etc\gui\scripts
C:\OSGeo4W\apps\grass\grass-6.4.3RC1\etc\python\grass\script
C:\OSGeo4W\apps\grass\grass-6.4.3RC1\etc\wxpython
C:\OSGeo4W\apps\qgis\grass\scripts

Same applies for grass-dev version (7.0)

Change History (11)

comment:1 by martinl, 11 years ago

Component: DefaultPython

in reply to:  1 ; comment:2 by glynn, 11 years ago

Replying to martinl:

I noticed that majority of python files in GRASS distribution are with mixed tabs and spaces

I think that we need an option to resolve issues as "so what?". But invalid or wontfix would suffice here. Mixing tabs and spaces isn't inherently a problem, so long as no-one uses something other than 8 column tabs (which is a bad idea for so many reasons). Even if we "fix" it for now, it will just keep creeping back in. Not all GRASS developers work on GRASS and GRASS alone, and not all text editors can easily be configured to automatically use different conventions for different projects. So people will continue to commit changes which use their text editor's default indentation. The only permanent fix would be to configure the subversion server to automatically convert files on upload (although that can create problems of its own).

in reply to:  2 comment:3 by klo, 11 years ago

Replying to glynn:

I think that we need an option to resolve issues as "so what?". But invalid or wontfix would suffice here.

"So what?" is lousy approach considering that sed one liner can fix it, and I don't know of serious Python package on my system that mixes tabs and spaces. I reported and that's enough from me, as I don't plan to advocate PEP8, but this is not like line exceeds 80th column. Mixing causes problems especially in team work. Google it.

comment:4 by wenzeslaus, 11 years ago

The problem is that we use 4 spaces indentation which really does not fit with 8 spaces wide tabs. In my view, it causes confusion. Of course, you often realize where is the problem, but is it necessary? (The is similar for GRASS C code (#1663) but the problem for Python is more critical since we don't and cannot have auto-indent tool for Python.)

Moreover, the problem of white spaces in GRASS Python code is larger. Try to use pep8 tool, the result is really bad. I think that if there is some standard, we should keep it, at least the basic rules.

I understand the problem of editor settings. I encountered it, too. However, I also know the opposite problem. My Python editor is confused and complains about GRASS code and thus it is sometimes hard to use editor's code checking features.

in reply to:  4 comment:5 by zarch, 11 years ago

Replying to wenzeslaus:

Moreover, the problem of white spaces in GRASS Python code is larger. Try to use pep8 tool, the result is really bad. I think that if there is some standard, we should keep it, at least the basic rules.

+1

I understand the problem of editor settings. I encountered it, too. However, I also know the opposite problem. My Python editor is confused and complains about GRASS code and thus it is sometimes hard to use editor's code checking features.

+1

why not use a pre-commit check, using pep8 like:

https://gist.github.com/2903822 # pep8 + pyflakes http://widerin.org/blog/pep8-subversion-pre-commit-hook # only pep8

to force developers to use a standard?

Using a standard can help to have a consistent python code and make the code easier to read.

Pietro

ps: pep8 command could be use also to convert all the existing python code to comply the pep8 directive...

in reply to:  4 comment:6 by hellik, 11 years ago

Replying to wenzeslaus:

The problem is that we use 4 spaces indentation which really does not fit with 8 spaces wide tabs. In my view, it causes confusion. Of course, you often realize where is the problem, but is it necessary? (The is similar for GRASS C code (#1663) but the problem for Python is more critical since we don't and cannot have auto-indent tool for Python.)

maybe it's worth to have a look to the original issue(but maybe not related and worth for another ticket?):

http://lists.osgeo.org/pipermail/osgeo4w-dev/2012-December/001970.html

I don't know if this is relevant for this list, but while looking to patch
right file to stop QGIS GRASS plugin from spawning popup windows on every
command execution

as this can also be seen sometimes with osge4w/standalone-wingrass7

comment:7 by neteler, 11 years ago

Should this script be improved/GRASS'ified?: tools/reindent.py

in reply to:  description ; comment:8 by hamish, 11 years ago

Keywords: indent added

Replying to klo:

Mixing tabs and spaces is not good idea: http://www.python.org/dev/peps/pep-0008/#tabs-or-spaces

annoyingly, it asserts it is so but does not explain why. is it just that lots of people submit code after editing with braindead text editors, or is it something inherent in the python language which makes it so...?

Hamish

in reply to:  8 comment:9 by zarch, 11 years ago

Replying to hamish:

Replying to klo:

Mixing tabs and spaces is not good idea: http://www.python.org/dev/peps/pep-0008/#tabs-or-spaces

annoyingly, it asserts it is so but does not explain why. is it just that lots of people submit code after editing with braindead text editors, or is it something inherent in the python language which makes it so...?

The problem is the inconsistency.

Python cleaned the syntax as much as possible, and use the space to define the code block, using a mixed code can cause troubles [0,1] because you are not consistent, and it is prone to errors. Moreover in python3 file with mixed syntax will not work.

Personally I think that we must choose the style to use and add pre-commit check consistently.

Using tabs to indent make problems when you want to aligned with opening delimiter, like:

# Aligned with opening delimiter
foo = long_function_name(var_one, var_two,
                         var_three, var_four)

With spaces you are sure that the code look the same for all the developers and not affect external source code viewers (e.g. the likes of github, pastebin, etc).

Pietro

[0] http://www.jroller.com/larrywilliams/entry/mixing_tabs_and_spaces_in [1] http://enjoydoingitwrong.wordpress.com/2009/03/24/mixing-tabs-and-spaces-in-python/

in reply to:  8 comment:10 by glynn, 11 years ago

Replying to hamish:

Replying to klo:

Mixing tabs and spaces is not good idea: http://www.python.org/dev/peps/pep-0008/#tabs-or-spaces

annoyingly, it asserts it is so but does not explain why. is it just that lots of people submit code after editing with braindead text editors, or is it something inherent in the python language which makes it so...?

Python uses indentation to determine block boundaries, so indentation matters.

Some text editors treat tab stops as configurable, but there's no way for the Python interpreter to know how the editor is configured, so it assumes that tab stops are every 8 columns. If you use an editor which is set to use e.g. 4-column tabs, then a line indented by 4 spaces and a line indented by a tab will appear to be at the same indentation level, but Python will treat the latter as indented more than the former.

comment:11 by neteler, 8 years ago

Milestone: 6.4.36.4.6
Note: See TracTickets for help on using tickets.