bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

gnulib-tool.py: Make data structures more clear.


From: Collin Funk
Subject: gnulib-tool.py: Make data structures more clear.
Date: Tue, 16 Apr 2024 09:06:06 -0700
User-agent: Mozilla Thunderbird

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]