radon icon indicating copy to clipboard operation
radon copied to clipboard

Provide option to return raw metrics by block type (#192

Open BlaneG opened this issue 5 years ago • 6 comments

This is a first attempt at implementing a raw_visitor to provide metrics by block type.

It is lightly tested reusing some of the tests from test_raw.py, not all of which are currently passing. Just looking for some feedback @rubik before adding more tests.

ast.get_source_segement was implemented in Python 3.8 which makes it easy to reproduce a segment of the input code from an ast node.

BlaneG avatar Jul 02 '20 01:07 BlaneG

@BlaneG Hi Blane, thanks for the contribution! I was not aware of such addition in Python 3.8, I will check it out. The idea is definitely interesting. It's something I wanted to do from the start, but at the time it wasn't possible in an easy way.

rubik avatar Aug 03 '20 19:08 rubik

I think the idea is good. Now besides the minor comments I added above, two things are needed mainly:

  1. enable all the tests and ensure everything is passing
  2. enable this functionality only on Python versions where it's supported. Currently the tests on Travis are failing for this reason.

Then before merging, we'll have to format the code with black and isort (I added the commands to the Makefile).

rubik avatar Sep 12 '20 17:09 rubik

Hi @BlaneG, in order to have this PR merged, you need to fix the conflicts and ensure all tests are passing. Are you still working on it?

rubik avatar Mar 07 '21 10:03 rubik

I came across this open PR when I was searching to see if there was any progress on the topic - is there anything I can do to help?

miketheman avatar Mar 09 '23 23:03 miketheman

@miketheman Hi Mike. Sure, I can merge the PR once:

  • all conflicts with the master branch are solved
  • all tests pass

rubik avatar Mar 25 '23 15:03 rubik

There's a big showstopper for ast.get_source_segment: it doesn't roundtrip comments, which makes raw metrics wrong.

I'm investigating alternatives that assure a source code roundtrip, hopefully without having to resort to a CST.

devdanzin avatar Sep 03 '23 13:09 devdanzin