Fix performance in get_imports regexp#34298
Conversation
|
It is not a typo but still a small change with existing tests |
|
This looks good now! It should be simpler and more performant, and shouldn't change which strings it matches. I have one more suggestion, though. This regex looks like it would fail in cases when the strings "try" and "except" occur internally in some lines. This is probably very rare, but maybe we should change the regex to To be clear, this isn't a bug with your PR - it's an issue with the regex that already existed! |
Any regex should fail because even valid Python source is not defined by a regular language. """
try:
pass
except:
pass
"""
# code in multi line string literalShould I add it in another PR with more tests? This PR is more about performance improvements. Maybe we can wait for @gante to reply, because he wrote this lines. |
Rocketknight1
left a comment
There was a problem hiding this comment.
@AlekseyLobanov yes, good point - we can keep this PR simple. Want me to merge this one now, and if you think you can improve it further, you can open a new PR with added test(s)?
Can we just merge this for now? It will be great. |
|
Yes, the diff looks good to me. Thanks for the PR! |
* fix: Fix performance in get_imports regexp * Minimize get_imports content regexp
What does this PR do?
Just found that one of the Regular Expressions may be very slow (O(N^3)) on some special inputs.
Something like
'try:' + ' ' * 1500may take a lot of time. I replaced it with the same logic but fasterBefore submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
@gante