Hi Bruno,
On 4/4/24 5:48 AM, Bruno Haible wrote:
> The only difference between the two outputs is the message between the two
> stack traces. The latter sounds like a program bug; therefore I would say,
> let's use the chaining — just in order to clarify that the second exception
> is intended.
Ah, that makes sense. I didn't really think about the intent that the
warning might convey. I agree with your assessment.
The first patch attached fixes all of those warnings by adding the
explicit chaining.
> Oh, indeed there's a fallacy here: {} denotes a dict, not a set!
>
>>>> type({'x','y'})
> <class 'set'>
>>>> type({'x'})
> <class 'set'>
>>>> type({})
> <class 'dict'>
Yes, it seems the Python developers ran out of brackets to use and
decided that sets should be the odd ones out. :(
>>> type(())
> <class 'tuple'>
>>> type([])
> <class 'list'>
>>> type({})
> <class 'dict'>
>>> type(set())
> <class 'set'>
The second patch disables that warning and changes the one occurrence
of '{}' we have to 'dict()'.
> This is surprising enough that we should add to our coding style:
>
> - Never use the {} literal, because it denotes a dictionary,
> as opposed to {x}, {x,y}, etc., which denote sets.
I've also added this to comment to main.py.
CollinFrom 501173c3439e5607828c086d41eb1be2bd49ae8f Mon Sep 17 00:00:00 2001
From: Collin Funk <collin.fu...@gmail.com>
Date: Thu, 4 Apr 2024 14:56:12 -0700
Subject: [PATCH 1/2] gnulib-tool.py: Fix pylint 'raise-missing-from' warnings.
* pygnulib/*.py: Use explicit exception chaining so that stack trace
messages do not seem like bugs. See examples in:
<https://lists.gnu.org/archive/html/bug-gnulib/2024-04/msg00056.html>
---
ChangeLog | 7 +++++++
pygnulib/GLConfig.py | 20 ++++++++++----------
pygnulib/GLFileSystem.py | 20 ++++++++++----------
pygnulib/GLImport.py | 8 ++++----
pygnulib/GLTestDir.py | 12 ++++++------
5 files changed, 37 insertions(+), 30 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 536c69737e..3cceb4c60a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2024-04-04 Collin Funk <collin.fu...@gmail.com>
+
+ gnulib-tool.py: Fix pylint 'raise-missing-from' warnings.
+ * pygnulib/*.py: Use explicit exception chaining so that stack trace
+ messages do not seem like bugs. See examples in:
+ <https://lists.gnu.org/archive/html/bug-gnulib/2024-04/msg00056.html>
+
2024-04-04 Bruno Haible <br...@clisp.org>
Add serial numbers to *.m4 files that did not have them.
diff --git a/pygnulib/GLConfig.py b/pygnulib/GLConfig.py
index 8136d9b08c..121edf541e 100644
--- a/pygnulib/GLConfig.py
+++ b/pygnulib/GLConfig.py
@@ -539,9 +539,9 @@ class GLConfig:
for module in modules:
try: # Try to add each module
self.addModule(module)
- except TypeError:
+ except TypeError as exc:
self.table['modules'] = old_modules
- raise TypeError('each module must be a string')
+ raise TypeError('each module must be a string') from exc
except GLError:
self.table['modules'] = old_modules
raise
@@ -585,9 +585,9 @@ class GLConfig:
for module in modules:
try: # Try to add each module
self.addAvoid(module)
- except TypeError:
+ except TypeError as exc:
self.table['avoids'] = old_avoids
- raise TypeError('each module must be a string')
+ raise TypeError('each module must be a string') from exc
except GLError:
self.table['avoids'] = old_avoids
raise
@@ -630,9 +630,9 @@ class GLConfig:
for file in files:
try: # Try to add each file
self.addFile(file)
- except TypeError:
+ except TypeError as exc:
self.table['files'] = old_files
- raise TypeError('each file must be a string')
+ raise TypeError('each file must be a string') from exc
except GLError:
self.table['files'] = old_files
raise
@@ -688,9 +688,9 @@ class GLConfig:
for category in categories:
try: # Try to enable each category
self.enableInclTestCategory(category)
- except TypeError:
+ except TypeError as exc:
self.table['incl_test_categories'] = old_categories
- raise TypeError('each category must be one of TESTS integers')
+ raise TypeError('each category must be one of TESTS integers') from exc
else: # if type of categories is not list or tuple
raise TypeError('categories must be a list or a tuple, not %s'
% type(categories).__name__)
@@ -736,9 +736,9 @@ class GLConfig:
for category in categories:
try: # Try to enable each category
self.enableExclTestCategory(category)
- except TypeError:
+ except TypeError as exc:
self.table['excl_test_categories'] = old_categories
- raise TypeError('each category must be one of TESTS integers')
+ raise TypeError('each category must be one of TESTS integers') from exc
else: # if type of categories is not list or tuple
raise TypeError('categories must be a list or a tuple, not %s'
% type(categories).__name__)
diff --git a/pygnulib/GLFileSystem.py b/pygnulib/GLFileSystem.py
index a0cddd8903..e157578e54 100644
--- a/pygnulib/GLFileSystem.py
+++ b/pygnulib/GLFileSystem.py
@@ -120,8 +120,8 @@ class GLFileSystem:
command = 'patch -s "%s" < "%s" >&2' % (tempFile, diff_in_localdir)
try: # Try to apply patch
sp.check_call(command, shell=True)
- except sp.CalledProcessError:
- raise GLError(2, name)
+ except sp.CalledProcessError as exc:
+ raise GLError(2, name) from exc
result = (tempFile, True)
else:
result = (lookedupFile, False)
@@ -260,8 +260,8 @@ class GLFileAssistant:
else: # Move instead of linking.
try: # Try to move file
movefile(tmpfile, joinpath(destdir, rewritten))
- except Exception:
- raise GLError(17, original)
+ except Exception as exc:
+ raise GLError(17, original) from exc
else: # if self.config['dryrun']
print('Copy file %s' % rewritten)
@@ -299,8 +299,8 @@ class GLFileAssistant:
os.remove(backuppath)
try: # Try to replace the given file
movefile(basepath, backuppath)
- except Exception:
- raise GLError(17, original)
+ except Exception as exc:
+ raise GLError(17, original) from exc
if self.filesystem.shouldLink(original, lookedup) == CopyAction.Symlink \
and not tmpflag and filecmp.cmp(lookedup, tmpfile):
link_if_changed(lookedup, basepath)
@@ -313,8 +313,8 @@ class GLFileAssistant:
if os.path.exists(basepath):
os.remove(basepath)
copyfile(tmpfile, joinpath(destdir, rewritten))
- except Exception:
- raise GLError(17, original)
+ except Exception as exc:
+ raise GLError(17, original) from exc
else: # if self.config['dryrun']
if already_present:
print('Update file %s (backup in %s)' % (rewritten, backupname))
@@ -343,8 +343,8 @@ class GLFileAssistant:
try: # Try to copy lookedup file to tmpfile
copyfile(lookedup, tmpfile)
ensure_writable(tmpfile)
- except Exception:
- raise GLError(15, lookedup)
+ except Exception as exc:
+ raise GLError(15, lookedup) from exc
# Don't process binary files with sed.
if not (original.endswith(".class") or original.endswith(".mo")):
transformer = None
diff --git a/pygnulib/GLImport.py b/pygnulib/GLImport.py
index 5cd39f66a3..c6d1a758a4 100644
--- a/pygnulib/GLImport.py
+++ b/pygnulib/GLImport.py
@@ -1018,8 +1018,8 @@ AC_DEFUN([%s_FILE_LIST], [\n''' % macro_prefix
if not self.config['dryrun']:
try: # Try to create directory
os.makedirs(directory)
- except Exception:
- raise GLError(13, directory)
+ except Exception as exc:
+ raise GLError(13, directory) from exc
else: # if self.config['dryrun']
print('Create directory %s' % directory)
@@ -1043,8 +1043,8 @@ AC_DEFUN([%s_FILE_LIST], [\n''' % macro_prefix
if os.path.exists(backup):
os.remove(backup)
movefile(path, '%s~' % path)
- except Exception:
- raise GLError(14, file)
+ except Exception as exc:
+ raise GLError(14, file) from exc
else: # if self.config['dryrun']
print('Remove file %s (backup in %s~)' % (path, path))
filetable['removed'] += [file]
diff --git a/pygnulib/GLTestDir.py b/pygnulib/GLTestDir.py
index 51c6ee3193..829e26fb3f 100644
--- a/pygnulib/GLTestDir.py
+++ b/pygnulib/GLTestDir.py
@@ -75,12 +75,12 @@ def _patch_test_driver() -> None:
command = f'patch {test_driver} < {diff}'
try:
result = sp.call(command, shell=True, stdout=sp.DEVNULL, stderr=sp.DEVNULL)
- except OSError:
+ except OSError as exc:
if isfile(f'{test_driver}.orig'):
os.remove(f'{test_driver}.orig')
if isfile(f'{test_driver}.rej'):
os.remove(f'{test_driver}.rej')
- raise GLError(20, None)
+ raise GLError(20, None) from exc
if result == 0:
patched = True
break
@@ -112,8 +112,8 @@ class GLTestDir:
if not os.path.exists(self.testdir):
try: # Try to create directory
os.mkdir(self.testdir)
- except Exception:
- raise GLError(19, self.testdir)
+ except Exception as exc:
+ raise GLError(19, self.testdir) from exc
self.emitter = GLEmiter(self.config)
self.filesystem = GLFileSystem(self.config)
self.modulesystem = GLModuleSystem(self.config)
@@ -900,8 +900,8 @@ class GLMegaTestDir:
if not os.path.exists(self.megatestdir):
try: # Try to create directory
os.mkdir(self.megatestdir)
- except Exception:
- raise GLError(19, self.megatestdir)
+ except Exception as exc:
+ raise GLError(19, self.megatestdir) from exc
self.emitter = GLEmiter(self.config)
self.filesystem = GLFileSystem(self.config)
self.modulesystem = GLModuleSystem(self.config)
--
2.44.0
From 97a978c99477fc7b236d5dbc49fb6a5955193a62 Mon Sep 17 00:00:00 2001
From: Collin Funk <collin.fu...@gmail.com>
Date: Thu, 4 Apr 2024 15:29:50 -0700
Subject: [PATCH 2/2] gnulib-tool.py: Ignore 'use-dict-literal' warnings.
* pygnulib/.pylintrc: Don't emit warning messages suggesting that
'dict()' be converted to '{}'. This literal can be mistaken for sets,
see discussion here:
<https://lists.gnu.org/archive/html/bug-gnulib/2024-04/msg00054.html>
* pygnulib/main.py: Document this convention in coding style section.
* pygnulib/GLFileSystem.py (GLFileAssistant.__init__): Convert an
occurrence of '{}' to 'dict()'.
---
ChangeLog | 11 +++++++++++
pygnulib/.pylintrc | 2 +-
pygnulib/GLFileSystem.py | 2 +-
pygnulib/main.py | 2 ++
4 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 3cceb4c60a..5a84f5a2c7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2024-04-04 Collin Funk <collin.fu...@gmail.com>
+
+ gnulib-tool.py: Ignore 'use-dict-literal' warnings.
+ * pygnulib/.pylintrc: Don't emit warning messages suggesting that
+ 'dict()' be converted to '{}'. This literal can be mistaken for sets,
+ see discussion here:
+ <https://lists.gnu.org/archive/html/bug-gnulib/2024-04/msg00054.html>
+ * pygnulib/main.py: Document this convention in coding style section.
+ * pygnulib/GLFileSystem.py (GLFileAssistant.__init__): Convert an
+ occurrence of '{}' to 'dict()'.
+
2024-04-04 Collin Funk <collin.fu...@gmail.com>
gnulib-tool.py: Fix pylint 'raise-missing-from' warnings.
diff --git a/pygnulib/.pylintrc b/pygnulib/.pylintrc
index 15e2e8e800..102accb37d 100644
--- a/pygnulib/.pylintrc
+++ b/pygnulib/.pylintrc
@@ -1,7 +1,7 @@
# .pylintrc
[MESSAGES CONTROL]
-disable=C0103,C0114,C0121,C0123,C0209,C0301,C0302,R0902,R0912,R0913,R0914,R0915,R1705,R1702,R1720
+disable=C0103,C0114,C0121,C0123,C0209,C0301,C0302,R0902,R0912,R0913,R0914,R0915,R1705,R1702,R1720,R1735
# Local Variables:
# mode: conf
diff --git a/pygnulib/GLFileSystem.py b/pygnulib/GLFileSystem.py
index e157578e54..551ae177e4 100644
--- a/pygnulib/GLFileSystem.py
+++ b/pygnulib/GLFileSystem.py
@@ -154,7 +154,7 @@ class GLFileSystem:
class GLFileAssistant:
'''GLFileAssistant is used to help with file processing.'''
- def __init__(self, config: GLConfig, transformers: dict[str, tuple[re.Pattern, str] | None] = {}) -> None:
+ def __init__(self, config: GLConfig, transformers: dict[str, tuple[re.Pattern, str] | None] = dict()) -> None:
'''Create GLFileAssistant instance.
config stores information shared between classes.
diff --git a/pygnulib/main.py b/pygnulib/main.py
index 233c79182e..4bf4da4279 100644
--- a/pygnulib/main.py
+++ b/pygnulib/main.py
@@ -52,6 +52,8 @@
# - Prefer the standard collection type hints over those provided in the
# typing module. The latter are deprecated and may be removed.
# Cf. <https://peps.python.org/pep-0585/>.
+# - Never use the {} literal, because it denotes a dictionary,
+# as opposed to {x}, {x,y}, etc., which denote sets.
# You can use this command to check the style:
# $ pycodestyle *.py
--
2.44.0
