On 23.9.2016 10:40, Petr Spacek wrote:
On 23.9.2016 07:28, Jan Cholasta wrote:
On 22.9.2016 16:39, Martin Basti wrote:
Hello all,
In 4.5, I would like to remove all unused variables from code and enable
pylint check. Due to big amount of unused variables in the code this
will be longterm effort.
Why this?:
* better code readability
* removing dead code
* unused variable may uncover potential bug
It is clear what to do with unused assignments, but I need an agreement
what to do with unpacking or iteration with unused variables
For example:
for name, surname, gender in (('Martin', 'Basti', 'M'), ):
name, surname, gender = user['mbasti']
Where 'surname' is unused
Pylint will detect surname as unused variable and we have to agree on a
way how to tell pylint that this variable is unused on purpose:
1)
(
name,
surname, # pylint: disable=unused-variable
gender
) = user['mbasti']
I dont like this approach
+1
2)
Use defined keyword: 'dummy' is default in pylint, we can set our own,
like ignored, unused
name, dummy, gender = user['mbasti']
-1, not visible enough.
3)
use a prefix for unused variables: '_' or 'ignore_'
name, _surname, gender = user['mbasti']
This. We have already been using it in new code for quite some time, and it's
common in other Python projects too. Don't reinvent the wheel.
4)
we can combine all :)
For me the best is to have prefix '_' and 'dummy' keyword
Use '_dummy', it's both :-)
I like "__". If it is not acceptable for rest of the team, I would be okay
with _dummy.
I'm not a fan of "__" (because it's as "brilliant" as "___" or "aaaa"),
but if any local variable with "_" prefix is considered unused (i.e.
what I'm proposing), it would be perfectly legal.
I would even use _dummy multiple times:
name, _dummy, _dummy = user['mbasti']
so namespace is not polluted and garbage collector can do the right thing.
This is a pretty misguided argument if I may say so. First, I don't see
how locals namespace pollution could ever cause any issues, and even if
it did, the proper way to avoid it is to not write long functions with
many local variables. Second, removing a local variable does not
guarantee that the garbage collector will do anything (because garbage
collection is not always deterministic), and even if it did, you should
be explicit about it and use the del statement.
Petr^2 Spacek
As first step I'll enable pylint check and disable it locally per module
where unused variables are, to avoid new regressions. Then I will fix it
module by module.
I'm open to suggestions
Martin^2
--
Jan Cholasta
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
