bug-gnulib@gnu.org mirror (unofficial)
 help / color / mirror / Atom feed
From: Collin Funk <collin.funk1@gmail.com>
To: bug-gnulib@gnu.org
Subject: gnulib-tool.py: Fix 'consider-using-with' pylint warnings.
Date: Thu, 4 Apr 2024 21:15:37 -0700	[thread overview]
Message-ID: <8a8630b8-d49e-44ea-a130-a41fe26a49c6@gmail.com> (raw)

[-- Attachment #1: Type: text/plain, Size: 1456 bytes --]

Here is two small patches to fix some pylint warnings. The first is
'consider-using-with'. This recommends to use context managers that
handle cleanup for you. In this case two Popen calls, but here is an
example with open():

     with open(file_name, 'r') as file:
         data = file.read()
     # File is closed for us.

is cleaner than:

     file = open(file_name, 'r')
     data = file.read()
     file.close()

In the case of Popen, "Popen objects are supported as context managers
via the with statement: on exit, standard file descriptors are closed,
and the process is waited for [1]."

I've just changed these to 'sp.run()' since that function deals with
everything for us and is recommended.

I've left it wrapped in os.chdir() calls since I rather convert all
occurrences of those to sp.run(cwd=directory) in one patch. Things seem
less likely to break that way.

The second is 'consider-using-set-comprehension'. Instead of calling
set() on a list comprehension we can just use a set comprehension.
I've additionally simplified this case:

-            version = sorted(set([ float(version)
-                                   for version in versions ]))[-1]
+            version = max({ float(version)
+                            for version in versions })

The previous method is just a less clear way of finding the max:

    print([1, 2, 3, 4][-1])
    4

[1] https://docs.python.org/3/library/subprocess.html#popen-constructor

Collin

[-- Attachment #2: 0001-gnulib-tool.py-Fix-consider-using-with-pylint-warnin.patch --]
[-- Type: text/x-patch, Size: 2141 bytes --]

From 7cd6b490d374077b67b5f82f7f1333b974ba5a30 Mon Sep 17 00:00:00 2001
From: Collin Funk <collin.funk1@gmail.com>
Date: Thu, 4 Apr 2024 20:32:55 -0700
Subject: [PATCH 1/2] gnulib-tool.py: Fix 'consider-using-with' pylint
 warnings.

* pygnulib/GLModuleSystem.py (GLModuleSystem.list): Use run() instead of
Popen() from the subprocess module. This function handles cleanup
internally instead of as a context manager via the 'with' statement.
---
 ChangeLog                  | 7 +++++++
 pygnulib/GLModuleSystem.py | 6 ++----
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 64f1af1000..0238480144 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2024-04-04  Collin Funk  <collin.funk1@gmail.com>
+
+	gnulib-tool.py: Fix 'consider-using-with' pylint warnings.
+	* pygnulib/GLModuleSystem.py (GLModuleSystem.list): Use run() instead of
+	Popen() from the subprocess module. This function handles cleanup
+	internally instead of as a context manager via the 'with' statement.
+
 2024-04-04  Collin Funk  <collin.funk1@gmail.com>
 
 	posix-modules, all-modules: Fix --version output using git options.
diff --git a/pygnulib/GLModuleSystem.py b/pygnulib/GLModuleSystem.py
index 1f5d536fe3..98a21c4696 100644
--- a/pygnulib/GLModuleSystem.py
+++ b/pygnulib/GLModuleSystem.py
@@ -136,16 +136,14 @@ class GLModuleSystem:
 
         # Read modules from gnulib root directory.
         os.chdir(constants.DIRS['root'])
-        find = sp.Popen(find_args, stdout=sp.PIPE)
-        result += find.stdout.read().decode("UTF-8")
+        result += sp.run(find_args, text=True, capture_output=True, check=False).stdout
         os.chdir(DIRS['cwd'])
 
         # Read modules from local directories.
         if len(localpath) > 0:
             for localdir in localpath:
                 os.chdir(localdir)
-                find = sp.Popen(find_args, stdout=sp.PIPE)
-                result += find.stdout.read().decode("UTF-8")
+                result += sp.run(find_args, text=True, capture_output=True, check=False).stdout
                 os.chdir(DIRS['cwd'])
 
         listing = [ line
-- 
2.44.0


[-- Attachment #3: 0002-gnulib-tool.py-Fix-consider-using-set-comprehension-.patch --]
[-- Type: text/x-patch, Size: 2380 bytes --]

From a35d8fe1d79742add3054eed9e434c4ec39b0cb1 Mon Sep 17 00:00:00 2001
From: Collin Funk <collin.funk1@gmail.com>
Date: Thu, 4 Apr 2024 20:42:09 -0700
Subject: [PATCH 2/2] gnulib-tool.py: Fix 'consider-using-set-comprehension'
 warnings.

* pygnulib/GLImport.py (GLImport.prepare): Create a set directly instead
of creating a list and passing it to a call of set().
(GLImport.__init__): Likewise. Use max() instead of getting the last
index of a sorted list.
---
 ChangeLog            | 8 ++++++++
 pygnulib/GLImport.py | 8 ++++----
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 0238480144..6e4be3e76c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2024-04-04  Collin Funk  <collin.funk1@gmail.com>
+
+	gnulib-tool.py: Fix 'consider-using-set-comprehension' warnings.
+	* pygnulib/GLImport.py (GLImport.prepare): Create a set directly instead
+	of creating a list and passing it to a call of set().
+	(GLImport.__init__): Likewise. Use max() instead of getting the last
+	index of a sorted list.
+
 2024-04-04  Collin Funk  <collin.funk1@gmail.com>
 
 	gnulib-tool.py: Fix 'consider-using-with' pylint warnings.
diff --git a/pygnulib/GLImport.py b/pygnulib/GLImport.py
index c6d1a758a4..ee238a1615 100644
--- a/pygnulib/GLImport.py
+++ b/pygnulib/GLImport.py
@@ -107,8 +107,8 @@ class GLImport:
         pattern = re.compile(r'.*AC_PREREQ\((.*)\)', re.M)
         versions = cleaner(pattern.findall(data))
         if versions:
-            version = sorted(set([ float(version)
-                                   for version in versions ]))[-1]
+            version = max({ float(version)
+                            for version in versions })
             self.config.setAutoconfVersion(version)
             if version < 2.64:
                 raise GLError(4, version)
@@ -810,8 +810,8 @@ AC_DEFUN([%s_FILE_LIST], [\n''' % macro_prefix
         m4base = self.config['m4base']
         lgpl = self.config['lgpl']
         verbose = self.config['verbosity']
-        base_modules = sorted(set([ self.modulesystem.find(m)
-                                    for m in modules ]))
+        base_modules = sorted({ self.modulesystem.find(m)
+                                for m in modules })
 
         # Perform transitive closure.
         final_modules = self.moduletable.transitive_closure(base_modules)
-- 
2.44.0


             reply	other threads:[~2024-04-05  4:15 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-05  4:15 Collin Funk [this message]
2024-04-05  4:53 ` gnulib-tool.py: Use 'Any' instead of type unions in GLConfig Collin Funk
2024-04-05 13:02   ` Bruno Haible
2024-04-05 12:58 ` gnulib-tool.py: Fix 'consider-using-with' pylint warnings Bruno Haible
2024-04-05 22:13   ` Collin Funk
2024-04-06 13:41     ` gnulib-tool.py: lists vs. sets Bruno Haible
2024-04-06 21:06       ` Collin Funk
2024-04-07 11:56         ` Bruno Haible
2024-04-07 13:06           ` Collin Funk
2024-04-07 14:23             ` Python list micro-benchmarks Bruno Haible
2024-04-07 23:07               ` Collin Funk
2024-04-08  0:51                 ` Paul Eggert
2024-04-08  1:01                   ` Collin Funk
2024-04-08 14:23                 ` Bruno Haible
2024-04-08 21:08                   ` Collin Funk
2024-04-08 21:19                     ` Bruno Haible
2024-04-08 22:36                       ` Paul Eggert
2024-04-09  1:26                         ` Collin Funk
2024-04-09  0:09                       ` gnulib-tool.py: Prefer 'list.append(item)' over 'list += [item]' Collin Funk
2024-04-09  1:28                         ` Bruno Haible
2024-04-09  1:38                           ` Collin Funk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://lists.gnu.org/mailman/listinfo/bug-gnulib

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8a8630b8-d49e-44ea-a130-a41fe26a49c6@gmail.com \
    --to=collin.funk1@gmail.com \
    --cc=bug-gnulib@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).