Hi Collin,
> The first is W0707 [1]. It recommends chaining exceptions. So this:
>
> try: # Try to apply patch
> sp.check_call(command, shell=True)
> except sp.CalledProcessError:
> raise GLError(2, name)
>
> becomes:
>
> try: # Try to apply patch
> sp.check_call(command, shell=True)
> except sp.CalledProcessError as exc:
> raise GLError(2, name) from exc
>
> This is used for the exception backtrace messages.
Let's see what the effect is:
===================== foo.py ==================
import subprocess as sp
class GLError(Exception):
'''foo'''
def __init__(self, errno: int) -> None:
pass
try:
sp.check_call('boom', shell=True)
except sp.CalledProcessError as x:
raise GLError(2) from x
===============================================
This produces the output:
--------------------------------------------------------------------------------
/bin/sh: 1: boom: not found
Traceback (most recent call last):
File "/home/bruno/foo.py", line 9, in <module>
sp.check_call('boom', shell=True)
File "/usr/lib/python3.10/subprocess.py", line 369, in check_call
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command 'boom' returned non-zero exit status 127.
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/home/bruno/foo.py", line 11, in <module>
raise GLError(2) from x
__main__.GLError: 2
--------------------------------------------------------------------------------
And without the chaining, it is:
--------------------------------------------------------------------------------
/bin/sh: 1: boom: not found
Traceback (most recent call last):
File "/home/bruno/foo.py", line 9, in <module>
sp.check_call('boom', shell=True)
File "/usr/lib/python3.10/subprocess.py", line 369, in check_call
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command 'boom' returned non-zero exit status 127.
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/home/bruno/foo.py", line 11, in <module>
raise GLError(2)
__main__.GLError: 2
--------------------------------------------------------------------------------
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.
> The second warning is R1735 [1]. It suggests changing 'dict()' to
> '{}'. I am fine with making that change or silencing the warning.
>
> I prefer the appearance of '{}', but can understand why some might
> dislike it:
>
> Dictionary:
> var = {} # Empty dictionary.
> # {0: 'a', 1: 'b', 2: 'c'}
> var = {i : chr(ord('a') + i) for i in range(3)}
>
> Set:
> var = set() # We are stuck with this. :(
> # Might be mistaken for a dictionary at a quick glance?
> # {1, 2, 3}
> var = {value for value in [1, 1, 2, 2, 3]}
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'>
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.
Bruno
