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

Reply via email to