Patch 0001 changes GLModuleTable's unconditionals member from a
dictionary to a set.
Since we only use it to check membership in
GLModuleTable.addConditional() it is effectively a set anyways. Then
in GLModuleTable.addUnconditional() we have:
self.unconditionals.add(str(module))
instead of:
self.unconditionals[str(module)] = True
Since we only care about membership, I find the first more clear. The
second makes me wonder if the value at the key is important.
Patch 0002 is a small style change. It is very small so I don't think
it is necessary to add to our list. I prefer, and tend to see more
often:
if var not in list:
# do something
instead of:
if not var in list:
# do something
PEP 8 makes this recommendation for 'is not' and 'not ... is' for
similar reasons [1].
$ grep -E 'not[[:blank:]]+[[:graph:]]+[[:blank:]]+in[[:blank:]]+' *.py
GLEmiter.py: else: # if not str(module) in ['gnumakefile',
'maintainer-makefile']
GLEmiter.py: # "make check", because "make check" is not possible in
a cross-compiling
GLEmiter.py: # "make check", because "make check" is not possible in
a cross-compiling
GLError.py: 1: file does not exist in GLFileSystem: <file>
GLError.py: message = 'file does not exist in GLFileSystem: %s'
% repr(errinfo)
GLModuleSystem.py: if not str(module) in self.unconditionals:
GLModuleSystem.py: if not (m in main_modules_set and
m.getApplicability() == 'main') ]
Since we only have two occurrences of this, I think it is fine to
change. The first is in a comment and the second is right above two
occurrences of the preferred way of writing it:
if not str(module) in self.unconditionals:
# No unconditional dependency to the given module is known at this
point.
if str(module) not in self.dependers:
self.dependers[str(module)] = []
if str(parent) not in self.dependers[str(module)]:
self.dependers[str(module)].append(str(parent))
Note that I am not counting this line since the 'not' is doing more
work than negating the 'in' and the parentheses make that clear. :)
GLModuleSystem.py: if not (m in main_modules_set and
m.getApplicability() == 'main') ]
[1] https://peps.python.org/pep-0008/#programming-recommendations
Collin
