[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