Patchwork localrepo: use modern import

login
register
mail settings
Submitter Gregory Szorc
Date May 5, 2015, 4:50 a.m.
Message ID <6c62247d0b6c111a6228.1430801439@gps-mbp.local>
Download mbox | patch
Permalink /patch/8892/
State Deferred
Headers show

Comments

Gregory Szorc - May 5, 2015, 4:50 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1430800991 25200
#      Mon May 04 21:43:11 2015 -0700
# Branch stable
# Node ID 6c62247d0b6c111a62281af647028e11504e3fe9
# Parent  cc497780eaf9c191564c64b40ca549f706c62724
localrepo: use modern import

This is a canary patch to test the waters around dropping Python 2.4
support by introducing the modern import statement, which will make
Python 3 compatbility easier and increase readability.

Python 3 changes the behavior of "import" to only be absolute. In Python
2.x "import foo" could import a top-level package or a module from the
current package. This resulted in ambiguity when the name of a module
conflicted with the name of a package.

Python 2.5 introduced the "absolute_import" feature to __future__ to
enable Python 2.x to import like Python 3. Adopting "absolute_import"
makes dual compatibility with Python 3 easier to achieve. In my
opinion, it also makes code easier to read since there is no ambiguity
as to where a module is coming from. With a project of Mercurial's
size (~120 .py files), this can be a real concern, especially for
new contributors.

PEP-0328 also introduced the multiple line import statement. Simply
add parenthesis and you get more readable import statements. This
PEP also introduced relative imports ("from . import foo").

This patch switches localrepo.py to use the modern import statement.
It introduces multiple line imports. It switches imports to relative.
It enforces the use of absolute imports by using the "absolute_import"
feature from __future__. While I was here, I also reordered imports:
standard library first, mercurial second, each alphabetized within its
subset. This included having one module per line, which is widely
considered a best practice in the Python community. If this patch is
accepted, we should probably deploy a linter to enforce that files
with "absolute_import" use this new standardized import convention.

The choice of relative imports versus absolute imports ("from ." vs
"from mercurial.") is arbitrary. Relative imports reduce typing.
They also make it easier to rename a package. They can, however,
make refactoring difficult. If a module's level changes, it's relative
imports also typically need to be changed. But we don't use multiple
layers of modules very much, so it shouldn't be a major concern at this
time. It is trivial to auto-rewrite the code in the future should we
wish to change conventions.

Use of multiple line import and "absolute_import" break compatibility
with Python 2.4. Good riddance.
Augie Fackler - May 5, 2015, 1:56 p.m.
On Mon, May 04, 2015 at 09:50:39PM -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1430800991 25200
> #      Mon May 04 21:43:11 2015 -0700
> # Branch stable
> # Node ID 6c62247d0b6c111a62281af647028e11504e3fe9
> # Parent  cc497780eaf9c191564c64b40ca549f706c62724
> localrepo: use modern import

I like this (in spirit - I think the syntax is a bit ugly but oh
well), but in talking to mpm yesterday a blocker for dropping 2.4 is
having an automated build that runs 'make docker-centos5' and puts
that package someplace useful for our forsaken friends on centos5.

Once that happens, I've got a big stack that gets test-run-tests.t to
pass on 2.6, 2.7, and 3.5 from a single tree, which I'll want to get
submitted and in a continuous build.

>
> This is a canary patch to test the waters around dropping Python 2.4
> support by introducing the modern import statement, which will make
> Python 3 compatbility easier and increase readability.
>
> Python 3 changes the behavior of "import" to only be absolute. In Python
> 2.x "import foo" could import a top-level package or a module from the
> current package. This resulted in ambiguity when the name of a module
> conflicted with the name of a package.
>
> Python 2.5 introduced the "absolute_import" feature to __future__ to
> enable Python 2.x to import like Python 3. Adopting "absolute_import"
> makes dual compatibility with Python 3 easier to achieve. In my
> opinion, it also makes code easier to read since there is no ambiguity
> as to where a module is coming from. With a project of Mercurial's
> size (~120 .py files), this can be a real concern, especially for
> new contributors.
>
> PEP-0328 also introduced the multiple line import statement. Simply
> add parenthesis and you get more readable import statements. This
> PEP also introduced relative imports ("from . import foo").
>
> This patch switches localrepo.py to use the modern import statement.
> It introduces multiple line imports. It switches imports to relative.
> It enforces the use of absolute imports by using the "absolute_import"
> feature from __future__. While I was here, I also reordered imports:
> standard library first, mercurial second, each alphabetized within its
> subset. This included having one module per line, which is widely
> considered a best practice in the Python community. If this patch is
> accepted, we should probably deploy a linter to enforce that files
> with "absolute_import" use this new standardized import convention.
>
> The choice of relative imports versus absolute imports ("from ." vs
> "from mercurial.") is arbitrary. Relative imports reduce typing.
> They also make it easier to rename a package. They can, however,
> make refactoring difficult. If a module's level changes, it's relative
> imports also typically need to be changed. But we don't use multiple
> layers of modules very much, so it shouldn't be a major concern at this
> time. It is trivial to auto-rewrite the code in the future should we
> wish to change conventions.
>
> Use of multiple line import and "absolute_import" break compatibility
> with Python 2.4. Good riddance.
>
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -3,23 +3,59 @@
>  # Copyright 2005-2007 Matt Mackall <mpm@selenic.com>
>  #
>  # This software may be used and distributed according to the terms of the
>  # GNU General Public License version 2 or any later version.
> -from node import hex, nullid, short
> -from i18n import _
> +
> +from __future__ import absolute_import
> +
> +import errno
> +import inspect
> +import os
> +import time
>  import urllib
> -import peer, changegroup, subrepo, pushkey, obsolete, repoview
> -import changelog, dirstate, filelog, manifest, context, bookmarks, phases
> -import lock as lockmod
> -import transaction, store, encoding, exchange, bundle2
> -import scmutil, util, extensions, hook, error, revset
> -import match as matchmod
> -import merge as mergemod
> -import tags as tagsmod
> -from lock import release
> -import weakref, errno, os, time, inspect
> -import branchmap, pathutil
> -import namespaces
> +import weakref
> +
> +from . import (
> +    bookmarks,
> +    branchmap,
> +    bundle2,
> +    changegroup,
> +    changelog,
> +    context,
> +    dirstate,
> +    encoding,
> +    error,
> +    exchange,
> +    extensions,
> +    filelog,
> +    hook,
> +    lock as lockmod,
> +    manifest,
> +    match as matchmod,
> +    merge as mergemod,
> +    namespaces,
> +    obsolete,
> +    pathutil,
> +    peer,
> +    phases,
> +    pushkey,
> +    repoview,
> +    revset,
> +    scmutil,
> +    store,
> +    subrepo,
> +    tags as tagsmod,
> +    transaction,
> +    util,
> +)
> +from .i18n import _
> +from .lock import release
> +from .node import (
> +    hex,
> +    nullid,
> +    short,
> +)
> +
>  propertycache = util.propertycache
>  filecache = scmutil.filecache
>
>  class repofilecache(filecache):
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -3,23 +3,59 @@ 
 # Copyright 2005-2007 Matt Mackall <mpm@selenic.com>
 #
 # This software may be used and distributed according to the terms of the
 # GNU General Public License version 2 or any later version.
-from node import hex, nullid, short
-from i18n import _
+
+from __future__ import absolute_import
+
+import errno
+import inspect
+import os
+import time
 import urllib
-import peer, changegroup, subrepo, pushkey, obsolete, repoview
-import changelog, dirstate, filelog, manifest, context, bookmarks, phases
-import lock as lockmod
-import transaction, store, encoding, exchange, bundle2
-import scmutil, util, extensions, hook, error, revset
-import match as matchmod
-import merge as mergemod
-import tags as tagsmod
-from lock import release
-import weakref, errno, os, time, inspect
-import branchmap, pathutil
-import namespaces
+import weakref
+
+from . import (
+    bookmarks,
+    branchmap,
+    bundle2,
+    changegroup,
+    changelog,
+    context,
+    dirstate,
+    encoding,
+    error,
+    exchange,
+    extensions,
+    filelog,
+    hook,
+    lock as lockmod,
+    manifest,
+    match as matchmod,
+    merge as mergemod,
+    namespaces,
+    obsolete,
+    pathutil,
+    peer,
+    phases,
+    pushkey,
+    repoview,
+    revset,
+    scmutil,
+    store,
+    subrepo,
+    tags as tagsmod,
+    transaction,
+    util,
+)
+from .i18n import _
+from .lock import release
+from .node import (
+    hex,
+    nullid,
+    short,
+)
+
 propertycache = util.propertycache
 filecache = scmutil.filecache
 
 class repofilecache(filecache):