Radon as a flake8 plugin

[alecxe]

Would it make sense to make a simple wrapper around radon to make it flake8 pluggable (similar to mccabe)?

Thanks.

[rubik]

I think it would. As for now, I only added the option --radon-max-cc, given that --max-complexity was already taken by mccabe. Would you try it and let me know?

I made a couple of runs and it seems to work as expected. Would you use other options? (Like --no-assert, or others...)

[alecxe]

@rubik wow, that was quick.

First of all, flake8 started to see the radon checker once I've installed radon into the virtual env - this part works as expected.

The current problem is, I think, that radon is enabled even if no radon-specific command-line arguments are passed. For instance, mccabe has the --max-complexity=-1 by default:

parser.add_option('--max-complexity', default=-1, action='store',
                  type='int', help="McCabe complexity threshold")

and it terminates if max-complexity is less than 0:

def run(self):
    if self.max_complexity < 0:
        return

which makes mccabe inactive unless --max-complexity is explicitly set.

Thanks!

[rubik]

Wow, I feel dumb. Thanks for catching that bit. I also added --radon-no-assert as an experiment. I run flake8 on sympy and found reports of huge complexity (over 200) in test files. So no it's possible not to count assert statements.

The problem now is that one could supply --radon-no-assert and nothing would happen. It could be confusing. What do you think?

[alecxe]

Thanks for the quick fix.

Yeah, current behavior could be confusing. It doesn't sound good to have --radon-max-cc flag enabling the checker.

May be it makes sense to enable radon checker if any of the radon-specific options are set applying defaults for others..
Another options would be to have a special --radon-enabled flag, but this would probably be an over-complication.

Also, would it be possible to put the "Maintainability Index" under the control too?

[rubik]

I'll think about this, maybe there is a better solution.

Also, would it be possible to put the "Maintainability Index" under the control too?

It certainly would, but MI is a very experimental metric which almost no one uses and I added just for my own interest. While CC is objectively useful, there are some concerns about MI, which are summed up nicely in this well-researched blog post by Arie van Deursen:
http://avandeursen.com/2014/08/29/think-twice-before-using-the-maintainability-index/

I would therefore abstain from adding it to flake8.

[alecxe]

@rubik yeah, I agree, it is a quite controversial measurement. Makes sense.

[rubik]

I don't think there's a good way to handle it for the user. The best idea is to enable the checker when a radon specific option is set, but then consider this case:

$ flake8 --radon-no-assert ...

In this case the radon checker should be enabled, but what should the default be for max_cc? If it stays at -1 then everything is reported (bad). On the other hand we could guess a default value (like 10) but I don't like changing defaults on the fly without notifying anyone.

Since I think automatically changing defaults is even more confusing than having --radon-max-cc enabling the checker, I'll go with the latter option, and make it clear that it's a required option if one wants to use Radon.

[rubik]

I'll now write some docs on this and then I'll release version 1.2.2

[alecxe]

I still like the idea that any radon-specific option enables the checker - it just sounds logical. In case of:

$ flake8 --radon-no-assert

we can apply a default max_cc value (like 10). And I think an end user would understand it - in case --radon-no-assert is explicitly set, a user definitely wants a checker to work leaving all other settings in a default state.

I don't like changing defaults on the fly without notifying anyone.

We can try adding debug messages, like "Max complexity was not specified, using the default 10 value".

On the other hand, I guess --radon-max-cc is the main configuration option and it sounds okay to have it required for the plugin to become enabled..

[rubik]

And I think an end user would understand it - in case --radon-no-assert is explicitly set, a user definitely wants a checker to work leaving all other settings in a default state.

For one I don't know if it's possible to print a debug message, the plugin interface does not seem to support it. And furthermore what you are saying makes sense. A sensible default value would work (aside from -1, which is there only to disable the checker). I think 10 is a good one.

[rubik]

Version 1.2.2 has been released on PyPI.

[alecxe]

Thank you for the quick changes!