Patchwork [2,of,2] py3: make files use absolute_import

login
register
mail settings
Submitter Pulkit Goyal
Date July 5, 2016, 8:42 p.m.
Message ID <22abe104ec6ab6c34e81.1467751325@pulkit-goyal>
Download mbox | patch
Permalink /patch/15754/
State Accepted
Headers show

Comments

Pulkit Goyal - July 5, 2016, 8:42 p.m.
# HG changeset patch
# User Pulkit Goyal <7895pulkit@gmail.com>
# Date 1467748954 -19800
#      Wed Jul 06 01:32:34 2016 +0530
# Node ID 22abe104ec6ab6c34e819a53aef7561e4a1620b0
# Parent  3862789efd2393c6ed7bfb856521d45fe17de105
py3: make files use absolute_import

This patch adds absolute_import to two files in which it was difficult to follow
importing conventions. To prevent producing unstable outputs in the test-check-module-imports,
 the files are added to the list of bad files which import-checker will skip.
Martijn Pieters - July 7, 2016, 1:22 p.m.
On 5 July 2016 at 21:42, Pulkit Goyal <7895pulkit@gmail.com> wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1467748954 -19800
> #      Wed Jul 06 01:32:34 2016 +0530
> # Node ID 22abe104ec6ab6c34e819a53aef7561e4a1620b0
> # Parent  3862789efd2393c6ed7bfb856521d45fe17de105
> py3: make files use absolute_import
>
> This patch adds absolute_import to two files in which it was difficult to follow
> importing conventions. To prevent producing unstable outputs in the test-check-module-imports,
>  the files are added to the list of bad files which import-checker will skip.

What happens when these are not added to the import-checker blacklist?
Is there some unstable ordering somewhere?

> diff -r 3862789efd23 -r 22abe104ec6a i18n/check-translation.py
> --- a/i18n/check-translation.py Wed Jul 06 00:03:33 2016 +0530
> +++ b/i18n/check-translation.py Wed Jul 06 01:32:34 2016 +0530
> @@ -2,6 +2,8 @@
>  #
>  # check-translation.py - check Mercurial specific translation problems
>
> +from __future__ import absolute_import
> +
>  import polib
>  import re
>
> diff -r 3862789efd23 -r 22abe104ec6a tests/test-check-module-imports.t
> --- a/tests/test-check-module-imports.t Wed Jul 06 00:03:33 2016 +0530
> +++ b/tests/test-check-module-imports.t Wed Jul 06 01:32:34 2016 +0530
> @@ -163,6 +163,7 @@
>    > -X doc/gendoc.py \
>    > -X doc/hgmanpage.py \
>    > -X i18n/posplit \
> +  > -X i18n/check-translation.py \
>    > -X tests/test-hgweb-auth.py \
>    > -X tests/hypothesishelpers.py \
>    > -X tests/test-ctxmanager.py \
> @@ -173,6 +174,7 @@
>    > -X tests/test-check-module-imports.t \
>    > -X tests/test-commit-interactive.t \
>    > -X tests/test-contrib-check-code.t \
> +  > -X tests/test-demandimport.py \
>    > -X tests/test-extension.t \
>    > -X tests/test-hghave.t \
>    > -X tests/test-hgweb-no-path-info.t \
> diff -r 3862789efd23 -r 22abe104ec6a tests/test-check-py3-compat.t
> --- a/tests/test-check-py3-compat.t     Wed Jul 06 00:03:33 2016 +0530
> +++ b/tests/test-check-py3-compat.t     Wed Jul 06 01:32:34 2016 +0530
> @@ -8,9 +8,7 @@
>    hgext/fsmonitor/pywatchman/__init__.py requires print_function
>    hgext/fsmonitor/pywatchman/capabilities.py not using absolute_import
>    hgext/fsmonitor/pywatchman/pybser.py not using absolute_import
> -  i18n/check-translation.py not using absolute_import
>    setup.py not using absolute_import
> -  tests/test-demandimport.py not using absolute_import
>
>  #if py3exe
>    $ hg files 'set:(**.py)' | sed 's|\\|/|g' | xargs $PYTHON3 contrib/check-py3-compat.py
> diff -r 3862789efd23 -r 22abe104ec6a tests/test-demandimport.py
> --- a/tests/test-demandimport.py        Wed Jul 06 00:03:33 2016 +0530
> +++ b/tests/test-demandimport.py        Wed Jul 06 01:32:34 2016 +0530
> @@ -1,4 +1,4 @@
> -from __future__ import print_function
> +from __future__ import absolute_import, print_function
>
>  from mercurial import demandimport
>  demandimport.enable()
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Yuya Nishihara - July 7, 2016, 2:21 p.m.
On Thu, 7 Jul 2016 14:22:09 +0100, Martijn Pieters wrote:
> On 5 July 2016 at 21:42, Pulkit Goyal <7895pulkit@gmail.com> wrote:
> > # HG changeset patch
> > # User Pulkit Goyal <7895pulkit@gmail.com>
> > # Date 1467748954 -19800
> > #      Wed Jul 06 01:32:34 2016 +0530
> > # Node ID 22abe104ec6ab6c34e819a53aef7561e4a1620b0
> > # Parent  3862789efd2393c6ed7bfb856521d45fe17de105
> > py3: make files use absolute_import
> >
> > This patch adds absolute_import to two files in which it was difficult to follow
> > importing conventions. To prevent producing unstable outputs in the test-check-module-imports,
> >  the files are added to the list of bad files which import-checker will skip.
> 
> What happens when these are not added to the import-checker blacklist?
> Is there some unstable ordering somewhere?

Our import-checker gets confused if a module is imported relative to the
main script. In the following example, "polib" is considered a local module,
but reordering "polib" and "re" would raise another warning.

> > +from __future__ import absolute_import
> > +
> >  import polib
> >  import re

We'll need to fix the import-checker.
Martijn Pieters - July 7, 2016, 5:01 p.m.
On 7 July 2016 at 15:21, Yuya Nishihara <yuya@tcha.org> wrote:
>> What happens when these are not added to the import-checker blacklist?
>> Is there some unstable ordering somewhere?
>
> Our import-checker gets confused if a module is imported relative to the
> main script. In the following example, "polib" is considered a local module,
> but reordering "polib" and "re" would raise another warning.

Thanks for explaining!
Pulkit Goyal - July 7, 2016, 5:43 p.m.
> What happens when these are not added to the import-checker blacklist?
> Is there some unstable ordering somewhere?

Yeah in case of test-demanimport.py, we import a module more than one
time so we have unstable ordering over there too.
Pulkit Goyal - Oct. 4, 2016, 2:07 p.m.
Any reason why this was left, need work on import checker?
On Jul 7, 2016 11:13 PM, "Pulkit Goyal" <7895pulkit@gmail.com> wrote:

> > What happens when these are not added to the import-checker blacklist?
> > Is there some unstable ordering somewhere?
>
> Yeah in case of test-demanimport.py, we import a module more than one
> time so we have unstable ordering over there too.
>
Yuya Nishihara - Oct. 4, 2016, 2:16 p.m.
On Tue, 4 Oct 2016 19:37:36 +0530, Pulkit Goyal wrote:
> Any reason why this was left, need work on import checker?

import checker is confused by "polib" vs "re".

Patch

diff -r 3862789efd23 -r 22abe104ec6a i18n/check-translation.py
--- a/i18n/check-translation.py	Wed Jul 06 00:03:33 2016 +0530
+++ b/i18n/check-translation.py	Wed Jul 06 01:32:34 2016 +0530
@@ -2,6 +2,8 @@ 
 #
 # check-translation.py - check Mercurial specific translation problems
 
+from __future__ import absolute_import
+
 import polib
 import re
 
diff -r 3862789efd23 -r 22abe104ec6a tests/test-check-module-imports.t
--- a/tests/test-check-module-imports.t	Wed Jul 06 00:03:33 2016 +0530
+++ b/tests/test-check-module-imports.t	Wed Jul 06 01:32:34 2016 +0530
@@ -163,6 +163,7 @@ 
   > -X doc/gendoc.py \
   > -X doc/hgmanpage.py \
   > -X i18n/posplit \
+  > -X i18n/check-translation.py \
   > -X tests/test-hgweb-auth.py \
   > -X tests/hypothesishelpers.py \
   > -X tests/test-ctxmanager.py \
@@ -173,6 +174,7 @@ 
   > -X tests/test-check-module-imports.t \
   > -X tests/test-commit-interactive.t \
   > -X tests/test-contrib-check-code.t \
+  > -X tests/test-demandimport.py \
   > -X tests/test-extension.t \
   > -X tests/test-hghave.t \
   > -X tests/test-hgweb-no-path-info.t \
diff -r 3862789efd23 -r 22abe104ec6a tests/test-check-py3-compat.t
--- a/tests/test-check-py3-compat.t	Wed Jul 06 00:03:33 2016 +0530
+++ b/tests/test-check-py3-compat.t	Wed Jul 06 01:32:34 2016 +0530
@@ -8,9 +8,7 @@ 
   hgext/fsmonitor/pywatchman/__init__.py requires print_function
   hgext/fsmonitor/pywatchman/capabilities.py not using absolute_import
   hgext/fsmonitor/pywatchman/pybser.py not using absolute_import
-  i18n/check-translation.py not using absolute_import
   setup.py not using absolute_import
-  tests/test-demandimport.py not using absolute_import
 
 #if py3exe
   $ hg files 'set:(**.py)' | sed 's|\\|/|g' | xargs $PYTHON3 contrib/check-py3-compat.py
diff -r 3862789efd23 -r 22abe104ec6a tests/test-demandimport.py
--- a/tests/test-demandimport.py	Wed Jul 06 00:03:33 2016 +0530
+++ b/tests/test-demandimport.py	Wed Jul 06 01:32:34 2016 +0530
@@ -1,4 +1,4 @@ 
-from __future__ import print_function
+from __future__ import absolute_import, print_function
 
 from mercurial import demandimport
 demandimport.enable()