[RFC/PATCH v2] CodingGuidelines: add Python coding guidelines

[RFC/PATCH v2] CodingGuidelines: add Python coding guidelines

[John Keeping]

These are kept short by simply deferring to PEP-8.  Most of the Python
code in Git is already very close to this style (some things in contrib/
are not).

Rationale for version suggestions:

 - Amongst the noise in [1], there isn't any disagreement about using
   2.6 as a base (see also [2]), although Brandon Casey recently added
   support for 2.4 and 2.5 to git-p4 [3].

 - Restricting ourselves to 2.6+ makes aiming for Python 3 compatibility
   significantly easier [4].

 - Advocating Python 3 support in all scripts is currently unrealistic
   because:

     - 'p4 -G' provides output in a format that is very hard to use with
       Python 3 (and its documentation claims Python 3 is unsupported).

     - Mercurial does not support Python 3.

     - Bazaar does not support Python 3.

 - But we should try to make new scripts compatible with Python 3
   because all new Python development is happening on version 3 and the
   Python community will eventually stop supporting Python 2 [5].

 - Python 3.1 is required to support the 'surrogateescape' error handler
   for encoding/decodng filenames to/from Unicode strings and Python 3.0
   is not longer supported.

[1] http://thread.gmane.org/gmane.comp.version-control.git/210329
[2] http://article.gmane.org/gmane.comp.version-control.git/210429
[3] http://thread.gmane.org/gmane.comp.version-control.git/214579
[4] http://docs.python.org/3.3/howto/pyporting.html#try-to-support-python-2-6-and-newer-only
[5] http://www.python.org/dev/peps/pep-0404/

---
Changes since v1:

- Set 3.1 as the minimum Python 3 version

- Remove the section on Unicode literals - it just adds confusion and
  doesn't apply to the current code; we can deal with any issues if they
  ever arise.

 Documentation/CodingGuidelines | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 69f7e9b..db7a416 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -179,6 +179,19 @@ For C programs:
  - Use Git's gettext wrappers to make the user interface
    translatable. See "Marking strings for translation" in po/README.
 
+For Python scripts:
+
+ - We follow PEP-8 (http://www.python.org/dev/peps/pep-0008/).
+
+ - As a minimum, we aim to be compatible with Python 2.6 and 2.7.
+
+ - Where required libraries do not restrict us to Python 2, we try to
+   also be compatible with Python 3.1 and later.
+
+ - We use the 'b' prefix for bytes literals.  Note that even though
+   the Python documentation for version 2.6 does not mention this
+   prefix it is supported since version 2.6.0.
+
 Writing Documentation:
 
  Every user-visible change should be reflected in the documentation.
-- 
1.8.1.1.644.g4977e08

Re: [RFC/PATCH v2] CodingGuidelines: add Python coding guidelines

[Junio C Hamano]

John Keeping <john@keeping.me.uk> writes:

> Changes since v1:
>
> - Set 3.1 as the minimum Python 3 version
>
> - Remove the section on Unicode literals - it just adds confusion and
>   doesn't apply to the current code; we can deal with any issues if they
>   ever arise.
> ...
> + - We use the 'b' prefix for bytes literals.  Note that even though
> +   the Python documentation for version 2.6 does not mention this
> +   prefix it is supported since version 2.6.0.

Do we still need to single out the 'b' prefix?  Even if it were
necessary, I'd like to see it toned down a bit to make it clear that
most of the time you can write strings as strings without having to
worry about the "is it unicode string or a string string" mess.
Like

    - When you must make distinction between Unicode literals and
      byte string literals, it is OK to use 'b' prefix.  Even though
      ...

perhaps?

Re: [RFC/PATCH v2] CodingGuidelines: add Python coding guidelines

[John Keeping]

On Tue, Jan 29, 2013 at 11:34:31AM -0800, Junio C Hamano wrote:
> John Keeping <john@keeping.me.uk> writes:
> 
> > Changes since v1:
> >
> > - Set 3.1 as the minimum Python 3 version
> >
> > - Remove the section on Unicode literals - it just adds confusion and
> >   doesn't apply to the current code; we can deal with any issues if they
> >   ever arise.
> > ...
> > + - We use the 'b' prefix for bytes literals.  Note that even though
> > +   the Python documentation for version 2.6 does not mention this
> > +   prefix it is supported since version 2.6.0.
> 
> Do we still need to single out the 'b' prefix?  Even if it were
> necessary, I'd like to see it toned down a bit to make it clear that
> most of the time you can write strings as strings without having to
> worry about the "is it unicode string or a string string" mess.
> Like
> 
>     - When you must make distinction between Unicode literals and
>       byte string literals, it is OK to use 'b' prefix.  Even though
>       ...
> 
> perhaps?

Yeah, that's better.

I want to include it because if you look in the Python documentation
then you'll be misled into thinking that it's not available on 2.6 and
we will be supporting that for a while since it is the version included
in RHEL 6.


John

Re: [RFC/PATCH v2] CodingGuidelines: add Python coding guidelines

[Michael Haggerty]

On 01/29/2013 08:08 PM, John Keeping wrote:
> These are kept short by simply deferring to PEP-8.  Most of the Python
> code in Git is already very close to this style (some things in contrib/
> are not).
> 
> Rationale for version suggestions:
> 
>  - Amongst the noise in [1], there isn't any disagreement about using
>    2.6 as a base (see also [2]), although Brandon Casey recently added
>    support for 2.4 and 2.5 to git-p4 [3].
> 
>  - Restricting ourselves to 2.6+ makes aiming for Python 3 compatibility
>    significantly easier [4].
> 
>  - Advocating Python 3 support in all scripts is currently unrealistic
>    because:
> 
>      - 'p4 -G' provides output in a format that is very hard to use with
>        Python 3 (and its documentation claims Python 3 is unsupported).
> 
>      - Mercurial does not support Python 3.
> 
>      - Bazaar does not support Python 3.
> 
>  - But we should try to make new scripts compatible with Python 3
>    because all new Python development is happening on version 3 and the
>    Python community will eventually stop supporting Python 2 [5].
> 
>  - Python 3.1 is required to support the 'surrogateescape' error handler
>    for encoding/decodng filenames to/from Unicode strings and Python 3.0
>    is not longer supported.
> 
> [1] http://thread.gmane.org/gmane.comp.version-control.git/210329
> [2] http://article.gmane.org/gmane.comp.version-control.git/210429
> [3] http://thread.gmane.org/gmane.comp.version-control.git/214579
> [4] http://docs.python.org/3.3/howto/pyporting.html#try-to-support-python-2-6-and-newer-only
> [5] http://www.python.org/dev/peps/pep-0404/
> 
> ---
> Changes since v1:
> 
> - Set 3.1 as the minimum Python 3 version
> 
> - Remove the section on Unicode literals - it just adds confusion and
>   doesn't apply to the current code; we can deal with any issues if they
>   ever arise.
> 
>  Documentation/CodingGuidelines | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
> index 69f7e9b..db7a416 100644
> --- a/Documentation/CodingGuidelines
> +++ b/Documentation/CodingGuidelines
> @@ -179,6 +179,19 @@ For C programs:
>   - Use Git's gettext wrappers to make the user interface
>     translatable. See "Marking strings for translation" in po/README.
>  
> +For Python scripts:
> +
> + - We follow PEP-8 (http://www.python.org/dev/peps/pep-0008/).
> +
> + - As a minimum, we aim to be compatible with Python 2.6 and 2.7.
> +
> + - Where required libraries do not restrict us to Python 2, we try to
> +   also be compatible with Python 3.1 and later.
> +
> + - We use the 'b' prefix for bytes literals.  Note that even though
> +   the Python documentation for version 2.6 does not mention this
> +   prefix it is supported since version 2.6.0.
> +
>  Writing Documentation:
>  
>   Every user-visible change should be reflected in the documentation.
> 

Nit: s/it is supported/it has been supported/

I think this would be a good Python policy.

I would hate to junk up all Python code with things like

    ' '.encode('ascii')

though, so maybe we should establish a small Python library of
compatibility utilities (like a small "six").  It could contain b().

Another handy utility function could be

    def check_python_version(minimum_v2=0x02060000,
                             minimum_v3=0x03010000)

which checks our default Python requirements by default, but is
overrideable by specific scripts if they know that they can deal with
older Python versions.

But I haven't had time to think of where to put such a library, how to
install it, etc.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: [RFC/PATCH v2] CodingGuidelines: add Python coding guidelines

[John Keeping]

On Wed, Jan 30, 2013 at 11:05:10AM +0100, Michael Haggerty wrote:
> Nit: s/it is supported/it has been supported/

Thanks, I'll fix in the re-roll.

> I think this would be a good Python policy.
> 
> I would hate to junk up all Python code with things like
> 
>     ' '.encode('ascii')
> 
> though, so maybe we should establish a small Python library of
> compatibility utilities (like a small "six").  It could contain b().
> 
> Another handy utility function could be
> 
>     def check_python_version(minimum_v2=0x02060000,
>                              minimum_v3=0x03010000)
> 
> which checks our default Python requirements by default, but is
> overrideable by specific scripts if they know that they can deal with
> older Python versions.
> 
> But I haven't had time to think of where to put such a library, how to
> install it, etc.

If we want to go that route, I think restructuring the
"git_remote_helpers" directory and re-using its infrastructure for
installing the "Git Python modules" would be the way to go.  The
directory structure would become something like this:

    git/
    `-- python/
        |-- Makefile    # existing file pulled out of git_remote_helpers
        |-- < some new utility library >
        |-- git_remote_helpers
        |   |-- __init__.py
        |   |-- git
        |   |   |-- __init__.py
        |   |   |-- exporter.py
        |   |   |-- git.py
        |   |   |-- importer.py
        |   |   |-- non_local.py
        |   |   `-- repo.py
        |   `-- util.py
        |-- setup.cfg   # existing file pulled out of git_remote_helpers
        `-- setup.py    # existing file pulled out of git_remote_helpers


It looks like the GitPython project[1] as already taken the "git" module
name, so perhaps we should use "git_core" if we do introduce a new
module.

[1] http://pypi.python.org/pypi/GitPython


John

Re: [RFC/PATCH v2] CodingGuidelines: add Python coding guidelines

[Michael Haggerty]

On 01/30/2013 09:31 PM, John Keeping wrote:
> On Wed, Jan 30, 2013 at 11:05:10AM +0100, Michael Haggerty wrote:
>> [...] maybe we should establish a small Python library of
>> compatibility utilities (like a small "six"). [...]
>> But I haven't had time to think of where to put such a library, how to
>> install it, etc.
> 
> If we want to go that route, I think restructuring the
> "git_remote_helpers" directory and re-using its infrastructure for
> installing the "Git Python modules" would be the way to go.  The
> directory structure would become something like this:
> 
>     git/
>     `-- python/
>         |-- Makefile    # existing file pulled out of git_remote_helpers
>         |-- < some new utility library >
>         |-- git_remote_helpers
>         |   |-- __init__.py
>         |   |-- git
>         |   |   |-- __init__.py
>         |   |   |-- exporter.py
>         |   |   |-- git.py
>         |   |   |-- importer.py
>         |   |   |-- non_local.py
>         |   |   `-- repo.py
>         |   `-- util.py
>         |-- setup.cfg   # existing file pulled out of git_remote_helpers
>         `-- setup.py    # existing file pulled out of git_remote_helpers
> 
> 
> It looks like the GitPython project[1] as already taken the "git" module
> name, so perhaps we should use "git_core" if we do introduce a new
> module.
> 
> [1] http://pypi.python.org/pypi/GitPython

This sounds reasonable.  But not all Python code will go under the
"python" subdirectory, right?  For example, I am working on a Python
script that fits thematically under contrib/hooks.

OTOH (I'm thinking aloud here) it is probably a bad idea for a hook
script to depend on a Python module that is part of git itself.  Doing
so would make the hook script depend on a particular version of git (or
at least a version with a compatible Python module).  But users might be
reluctant to upgrade git just to install a hook script.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: [RFC/PATCH v2] CodingGuidelines: add Python coding guidelines

[John Keeping]

On Fri, Feb 01, 2013 at 09:39:39AM +0100, Michael Haggerty wrote:
> On 01/30/2013 09:31 PM, John Keeping wrote:
> > On Wed, Jan 30, 2013 at 11:05:10AM +0100, Michael Haggerty wrote:
> >> [...] maybe we should establish a small Python library of
> >> compatibility utilities (like a small "six"). [...]
> >> But I haven't had time to think of where to put such a library, how to
> >> install it, etc.
> > 
> > If we want to go that route, I think restructuring the
> > "git_remote_helpers" directory and re-using its infrastructure for
> > installing the "Git Python modules" would be the way to go.  The
> > directory structure would become something like this:
> > 
> >     git/
> >     `-- python/
> >         |-- Makefile    # existing file pulled out of git_remote_helpers
> >         |-- < some new utility library >
> >         |-- git_remote_helpers
> >         |   |-- __init__.py
> >         |   |-- git
> >         |   |   |-- __init__.py
> >         |   |   |-- exporter.py
> >         |   |   |-- git.py
> >         |   |   |-- importer.py
> >         |   |   |-- non_local.py
> >         |   |   `-- repo.py
> >         |   `-- util.py
> >         |-- setup.cfg   # existing file pulled out of git_remote_helpers
> >         `-- setup.py    # existing file pulled out of git_remote_helpers
> > 
> > 
> > It looks like the GitPython project[1] as already taken the "git" module
> > name, so perhaps we should use "git_core" if we do introduce a new
> > module.
> > 
> > [1] http://pypi.python.org/pypi/GitPython
> 
> This sounds reasonable.  But not all Python code will go under the
> "python" subdirectory, right?  For example, I am working on a Python
> script that fits thematically under contrib/hooks.

I was thinking of it as analagous with the "perl" directory that
currently exists.  So the "python" directory will contain library code
but scripts can live wherever is most appropriate.

One way of looking at it is: could the user want to have this installed
for all available versions of Python?  For a script, the answer is "no"
because they will call it and it will just run.  For libraries, you want
them to be available with whatever Python interpreter you happen to be
running (assuming that it is a version supported by the library).

> OTOH (I'm thinking aloud here) it is probably a bad idea for a hook
> script to depend on a Python module that is part of git itself.  Doing
> so would make the hook script depend on a particular version of git (or
> at least a version with a compatible Python module).  But users might be
> reluctant to upgrade git just to install a hook script.

I don't think such a dependency is a bad idea in the longer term.  If a
"Git Python library" is developed, then at some point most people who
have Git installed will have some version of that library - it becomes a
case of perhaps wanting to limit yourself to some subset of the library
rather than just not using it.

In fact, git_remote_helpers has been available since Git 1.7.0 and
contains a lot of functionality that is more generic than its name
suggests.


John

Re: [RFC/PATCH v2] CodingGuidelines: add Python coding guidelines

[Pete Wyckoff]

john@keeping.me.uk wrote on Fri, 01 Feb 2013 11:16 +0000:
> On Fri, Feb 01, 2013 at 09:39:39AM +0100, Michael Haggerty wrote:
> > On 01/30/2013 09:31 PM, John Keeping wrote:
> > > On Wed, Jan 30, 2013 at 11:05:10AM +0100, Michael Haggerty wrote:
> > >> [...] maybe we should establish a small Python library of
> > >> compatibility utilities (like a small "six"). [...]
> > >> But I haven't had time to think of where to put such a library, how to
> > >> install it, etc.
> > > 
> > > If we want to go that route, I think restructuring the
> > > "git_remote_helpers" directory and re-using its infrastructure for
> > > installing the "Git Python modules" would be the way to go.  The
> > > directory structure would become something like this:
> > > 
> > >     git/
> > >     `-- python/
> > >         |-- Makefile    # existing file pulled out of git_remote_helpers
> > >         |-- < some new utility library >
> > >         |-- git_remote_helpers
> > >         |   |-- __init__.py
> > >         |   |-- git
> > >         |   |   |-- __init__.py
> > >         |   |   |-- exporter.py
> > >         |   |   |-- git.py
> > >         |   |   |-- importer.py
> > >         |   |   |-- non_local.py
> > >         |   |   `-- repo.py
> > >         |   `-- util.py
> > >         |-- setup.cfg   # existing file pulled out of git_remote_helpers
> > >         `-- setup.py    # existing file pulled out of git_remote_helpers
> > > 
> > > 
> > > It looks like the GitPython project[1] as already taken the "git" module
> > > name, so perhaps we should use "git_core" if we do introduce a new
> > > module.
> > > 
> > > [1] http://pypi.python.org/pypi/GitPython
> > 
> > This sounds reasonable.  But not all Python code will go under the
> > "python" subdirectory, right?  For example, I am working on a Python
> > script that fits thematically under contrib/hooks.
> 
> I was thinking of it as analagous with the "perl" directory that
> currently exists.  So the "python" directory will contain library code
> but scripts can live wherever is most appropriate.
> 
> One way of looking at it is: could the user want to have this installed
> for all available versions of Python?  For a script, the answer is "no"
> because they will call it and it will just run.  For libraries, you want
> them to be available with whatever Python interpreter you happen to be
> running (assuming that it is a version supported by the library).
> 
> > OTOH (I'm thinking aloud here) it is probably a bad idea for a hook
> > script to depend on a Python module that is part of git itself.  Doing
> > so would make the hook script depend on a particular version of git (or
> > at least a version with a compatible Python module).  But users might be
> > reluctant to upgrade git just to install a hook script.
> 
> I don't think such a dependency is a bad idea in the longer term.  If a
> "Git Python library" is developed, then at some point most people who
> have Git installed will have some version of that library - it becomes a
> case of perhaps wanting to limit yourself to some subset of the library
> rather than just not using it.
> 
> In fact, git_remote_helpers has been available since Git 1.7.0 and
> contains a lot of functionality that is more generic than its name
> suggests.

This library idea would be a great help; there are 100-odd calls
to git in git-p4, and we've had to deal with getting the arguments
and parsing correct.  I'd happily switch to using git_core.

Probably some elements of GitPython can be used too.  I'm not so
interested in the raw database manipulation, but the command
wrappers look reasonable.

		-- Pete