Fix remove_class #50
Fix remove_class #50
Conversation
| pass | ||
| if True: | ||
| if False: | ||
| pass |
There was a problem hiding this comment.
Why this doc-string and this code? Is it to test dedenting? So please add a brief comment above.
| ( | ||
| 1, | ||
| 2, | ||
| ) |
There was a problem hiding this comment.
Same here: Why this fancy, non-PEP-8 formattted code?
| break | ||
| else: | ||
| i = len(s) | ||
| return s[:i] + s[i:-dedent] |
There was a problem hiding this comment.
Plese add a breif comment what this does
| leaf.prefix = safe_dedent(leaf.prefix) | ||
| elif leaf.type == token.INDENT: | ||
| leaf.value = leaf.value[:-dedent] | ||
| elif leaf.prefix[:1] in "\r\n": |
There was a problem hiding this comment.
| elif leaf.prefix[:1] in "\r\n": | |
| elif leaf.startswith(("\r", "\n")): |
There was a problem hiding this comment.
both are safe, and it turns out the second form is ~50% slower:
$ python -m timeit '""[:1] in "ab"'
5000000 loops, best of 5: 48.6 nsec per loop
$ python -m timeit '"".startswith(("a", "b"))'
5000000 loops, best of 5: 70.5 nsec per loop
$ python -m timeit '"b"[:1] in "ab"'
5000000 loops, best of 5: 49.8 nsec per loop
$ python -m timeit '"b".startswith(("a", "b"))'
5000000 loops, best of 5: 77.8 nsec per loop
$ python -m timeit --setup="s = 'x'*1000" 's[:1] in "ab'
5000000 loops, best of 5: 48.4 nsec per loop
$ python -m timeit --setup="s = 'x'*1000" 's.startswith(("a", "b"))'
5000000 loops, best of 5: 75.7 nsec per loop
There was a problem hiding this comment.
Anyhow, the later form is much more easy to understand. And there is not need to optimize for speed in this project, isn't it?
|
Thanks for the review, I amended with changes. Previously I put in too many cases while trying to understand show things were tokenised, so now they've been simplified and moved to their own fixture. |
This is to fix #49