[Freeipa-devel] pylint: remove unused variables

[Petr Spacek]

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 would even use _dummy multiple times:
> name, _dummy, _dummy = user['mbasti']
so namespace is not polluted and garbage collector can do the right thing.

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