Patchwork [1,of,3] import-checker: refactor sys.path prefix check (issue4129)

login
register
mail settings
Submitter Chris Jerdonek
Date Dec. 25, 2013, 12:08 a.m.
Message ID <CAOTb1wfawKYj27b7V2O12QiwQNmocz0U7Yko4ntg36bjjEC=KQ@mail.gmail.com>
Download mbox | patch
Permalink /patch/3236/
State Accepted
Headers show

Comments

Chris Jerdonek - Dec. 25, 2013, 12:08 a.m.
On Tue, Dec 24, 2013 at 2:52 PM, Augie Fackler <raf@durin42.com> wrote:
>
> On Dec 24, 2013, at 5:45 PM, Augie Fackler <raf@durin42.com> wrote:
>
>>
>> On Dec 23, 2013, at 12:34 AM, Chris Jerdonek <chris.jerdonek@gmail.com> wrote:
>>
>>>
>>>    for libpath in sys.path:
>>> -        # We want to walk everything in sys.path that starts with
>>> -        # either sys.prefix or sys.exec_prefix.
>>> -        if not (libpath.startswith(sys.prefix)
>>> -                or libpath.startswith(sys.exec_prefix)):
>>> +        # We want to walk everything in sys.path that starts with something
>>> +        # in stdlib_prefixes.
>>> +        for prefix in stdlib_prefixes:
>>> +            if libpath.startswith(prefix):
>>> +                break
>>> +        else:
>>>            continue
>>
>> This file already depends on 2.6isms (the ast module), so perhaps we could use any() here and avoid the slightly-awkward-to-me for/else? I think it might read more clearly.
>
> After looking at the rest of the series, I'm crewing it with my own follow-up patch to use any() on a genexp instead of the for/break/else/continue trick. Thanks for following up on this!

--Chris
Augie Fackler - Dec. 25, 2013, 12:09 a.m.
On Dec 24, 2013, at 7:08 PM, Chris Jerdonek <chris.jerdonek@gmail.com> wrote:

> diff --git a/contrib/import-checker.py b/contrib/import-checker.py
> --- a/contrib/import-checker.py
> +++ b/contrib/import-checker.py
> @@ -73,10 +73,7 @@
>    for libpath in sys.path:
>        # We want to walk everything in sys.path that starts with something
>        # in stdlib_prefixes.
> -        for prefix in stdlib_prefixes:
> -            if libpath.startswith(prefix):
> -                break
> -        else:
> +        if not any(libpath.startswith(p) for p in prefix):
> 
> Oh, you want this to be "for p in stdlib_prefixes" though.


Derp. Thanks, I'll push a fixed version of my change.

Patch

diff --git a/contrib/import-checker.py b/contrib/import-checker.py
--- a/contrib/import-checker.py
+++ b/contrib/import-checker.py
@@ -73,10 +73,7 @@ 
     for libpath in sys.path:
         # We want to walk everything in sys.path that starts with something
         # in stdlib_prefixes.
-        for prefix in stdlib_prefixes:
-            if libpath.startswith(prefix):
-                break
-        else:
+        if not any(libpath.startswith(p) for p in prefix):

Oh, you want this to be "for p in stdlib_prefixes" though.