Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use python tokenizer to identify single-line comments and docstrings. #106

Merged
merged 2 commits into from
Mar 14, 2017
Merged

Use python tokenizer to identify single-line comments and docstrings. #106

merged 2 commits into from
Mar 14, 2017

Conversation

ashanbrown
Copy link
Contributor

Perform the raw analysis using a single iterator.
Add test cases that cover line continuations and unusual docstrings.

@coveralls
Copy link

coveralls commented Mar 6, 2017 •
edited
Loading

Coverage Status

Coverage increased (+0.01%) to 98.281% when pulling 7c585d5 on ashanbrown:update-docstring-analysis into 52daa6c on rubik:master.

@ashanbrown ashanbrown force-pushed the update-docstring-analysis branch from 7c585d5 to 79548fd Compare March 6, 2017 06:30
@coveralls
Copy link

coveralls commented Mar 6, 2017 •
edited
Loading

Coverage Status

Coverage increased (+0.01%) to 98.281% when pulling 79548fd on ashanbrown:update-docstring-analysis into 52daa6c on rubik:master.

@ashanbrown
Copy link
Contributor Author

ashanbrown commented Mar 6, 2017 •
edited
Loading

Ciao @rubik . I was having trouble performing a raw analyze on a codebase so I rewrote much of the comment recognizing code from an early PR. In particular, the code I was having a problem with looked like this:

def function():
    multiline_with_equals_in_it = """ """
    pass

There is some existing code that tries to recognize the position of '=' in multi-line string assignments that makes assumptions that may not always be the case. I have replaced this code with code that uses the python tokenizer to recognize strings and comments rather than using a custom parser. I've also tried to reduce the number of times radon parses the code to a single time.

I have only changed one existing test case, which appeared to be expecting the wrong number of blank lines. I'm assuming we should count lines as blank whether or not they are inside multi-line strings.

Please let me know what I can do to get this into the codebase. Thank you.

ashanbrown
ashanbrown commented Mar 6, 2017
View reviewed changes
radon/tests/test_raw.py
@@ -187,7 +187,7 @@ def fib(n):
"""
if n <= 1: return 1 # otherwise it will melt the cpu
return fib(n - 2) + fib(n - 1)
''', (6, 9, 10, 2, 3, 1, 1)),
''', (6, 9, 10, 2, 3, 2, 1)),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that '2' is the correct value for number of blank lines in this code. I think previously the line in the multiline string had been excluded.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 98.281% when pulling dd0eb9e on ashanbrown:update-docstring-analysis into 52daa6c on rubik:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 98.281% when pulling dd0eb9e on ashanbrown:update-docstring-analysis into 52daa6c on rubik:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 98.424% when pulling 220ab6b on ashanbrown:update-docstring-analysis into 52daa6c on rubik:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 98.424% when pulling 220ab6b on ashanbrown:update-docstring-analysis into 52daa6c on rubik:master.

@coveralls
Copy link

coveralls commented Mar 6, 2017 •
edited
Loading

Coverage Status

Coverage increased (+0.2%) to 98.424% when pulling 220ab6b on ashanbrown:update-docstring-analysis into 52daa6c on rubik:master.

@ashanbrown ashanbrown force-pushed the update-docstring-analysis branch from 220ab6b to 1305669 Compare March 6, 2017 15:51
@coveralls
Copy link

coveralls commented Mar 6, 2017 •
edited
Loading

Coverage Status

Coverage increased (+0.2%) to 98.424% when pulling 1305669 on ashanbrown:update-docstring-analysis into 52daa6c on rubik:master.

1 similar comment
@coveralls
Copy link

coveralls commented Mar 6, 2017 •
edited
Loading

Coverage Status

Coverage increased (+0.2%) to 98.424% when pulling 1305669 on ashanbrown:update-docstring-analysis into 52daa6c on rubik:master.

@ashanbrown ashanbrown changed the title Use AST to find single-line comments and docstrings. Use python tokenizer to identify single-line comments and docstrings. Mar 7, 2017
Andrew S. Brown added 2 commits March 7, 2017 10:31
Use python tokenizer to identify single-line comments and docstrings.
4b7594e 
Perform the raw analysis using a single iterator.
Add test cases that cover line continuations and unusual docstrings.
Require py.test 2.7 or more.
5f05f1b
@ashanbrown ashanbrown force-pushed the update-docstring-analysis branch from 1305669 to 5f05f1b Compare March 7, 2017 18:32
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 98.424% when pulling 5f05f1b on ashanbrown:update-docstring-analysis into 52daa6c on rubik:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 98.424% when pulling 5f05f1b on ashanbrown:update-docstring-analysis into 52daa6c on rubik:master.

@coveralls
Copy link

coveralls commented Mar 7, 2017 •
edited
Loading

Coverage Status

Coverage increased (+0.2%) to 98.424% when pulling 5f05f1b on ashanbrown:update-docstring-analysis into 52daa6c on rubik:master.

@rubik
Copy link
Owner

rubik commented Mar 7, 2017 •
edited
Loading

Thanks Andrew! I'll review this tomorrow but at a cursory glance it looks good. The code you provided definitely crashes the analysis, so something will have to be done for sure.

@rubik
Copy link
Owner

rubik commented Mar 14, 2017

Sorry for the delay Andrew! The PR looks good and I'm merging it. Thanks again!

@rubik rubik merged commit a34f0b6 into rubik:master Mar 14, 2017
@ashanbrown
Copy link
Contributor Author

Thanks @rubik . Please let me know if there are any reported issues with this change. I'll be happy to try to fix them.

@ashanbrown ashanbrown deleted the update-docstring-analysis branch March 27, 2017 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ashanbrown @coveralls @rubik