[openstack-dev] [all] cyclomatic complexity check in flake8 & Python 3

Zane Bitter zbitter at redhat.com
Mon Dec 18 19:59:35 UTC 2017


On 12/12/17 08:40, Sean Dague wrote:
> On 12/12/2017 08:08 AM, Thierry Carrez wrote:
>> Zane Bitter wrote:
> <snip>
>>> Here's how the allegedly most complex function in Heat[5] shakes out,
>>> for example:
>>>
>>>    mccabe  py27  py36
>>>    0.2.1    14     6
>>>    0.3.1    23    23
>>>    0.6.1    23    23
>>>
>>> I don't have a solution to suggest, but I burned most of my day digging
>>> into this so I just wanted to let y'all know how screwed we are.
>> Thanks for digging into this, Zane. I'd say we have two options, based
>> on how useful running those tests is:
>>
>> 1/ If they are useful, we should bump the version, at the same time
>> adjusting max-complexity per repo to match the most complex function
>> (with some slack factor). Encourage more projects to use cyclomatic
>> complexity checks and fix the biggest offenders using cycle goals.
>>
>> 2/ If they are not useful, just drop cyclomatic complexity tests overall
>> rather than relying on wrong results and wasting CPU cycles
>>
>> So I'd like to hear from the 60+ repos on whether using those tests lead
>> to any good results :)
> 
> I don't know how useful these end up being (#1), though I've seen
> patches from time to time with people trying to reduce them.

One problem with the theory is that the max complexity is pretty much 
always going to be dominated by a couple of tent-pole values, with 
everything else free to accumulate complexity under the radar. (I 
apologise for the preceding mixed metaphor.) If you could specify 
per-file exceptions then you could at least use it in the way that's 
intended (whether that's useful or not is still up for debate - 
personally I'm in Thomas's "not" camp), but afaik you can't.

> The current max in Nova is 35. Moving to mccabe 0.6.1 generates 2 errors:
> 
> Running flake8 on all files
> ./nova/compute/manager.py:763:5: C901 'ComputeManager._init_instance' is
> too complex (37)
> ./nova/tests/functional/api_samples_test_base.py:144:5: C901
> 'ApiSampleTestBase._compare_result' is too complex (36)

There are some good fixes in 0.6.1 that are pushing the scores up, like 
now counting complexity that occurs inside of the 'else' part of 
try/except/else, but I suspect that the vast majority of the increase is 
due to one giant, honking regression:

https://github.com/PyCQA/mccabe/issues/27#issuecomment-352535619

so TBH I wouldn't recommend updating to any current release. Only if 
somebody is motivated to actually fix that bug might it be worth 
considering.

cheers,
Zane.

> I honestly think this is one of the things that you declare a flag day a
> couple weeks out, warn everyone they are going to have to adjust their
> max, and move forward. It's a very easy fix when pep8 starts failing people.
> 
> 	-Sean
> 




More information about the OpenStack-dev mailing list