shorter and faster version extraction from __init__.py

[deronnax]

@sigmavirus24 I though it could be improved a bit. Tell me what you think.

[deronnax]

if you think readability has been tampered too much, I'll make a pull request with this less-pure-but-softer version:

-    for line in fd:
-        m = reg.match(line)
-        if m:
-            version = m.group(1)
-            break
+    version = reg.search(fd.read())

[Lukasa]

No objections from me. 👍

[sigmavirus24]

Do you have benchmarks? Also, why are we optimizing something that is run at most once by a user (during pip install)?

[deronnax]

It's not optimizing. It's making it simpler. And thus that it's simpler also makes faster.
What you did was opening the file, buffering it, crawling it line by line (triggering a search for line terminators whereas we don't care about) and applying a regex on each line.
What I do is opening and reading the whole file and applying the regex on the content.
This uninteresting microbenchmark shows us it's 3 time faster, but that's not my point. It's simpler, that's what matter :)

[sigmavirus24]

Since we're not splitting on line terminators anymore and lazily ejecting when we've found what we're looking for, the regular expression needs to be updated. We want to make sure we don't grab any cruft from a separate line that has version in it.

match will automatically add the equivalent of ^ to the beginning of the regular expression. search does not. We don't want anything where there's a line that looks like

# We want __version__ = "some string"
...
__version__ = "2.6.1"

Because we'll get the wrong result. Please fix the expression to check for line beginnings and endings.

[sigmavirus24]

Either way, this change is incomplete in its current form. I've left feedback on what is necessary to fix it.

[deronnax]

Yeah, sorry for the cardet, I thought about it and forgot.
I think using the dollar and thus requiring the "version" line ends with a newline is a bad idea : it might end with a whitespace or a comment, it's valid.
I think the cardet suffices. And it thus behaves the same as your version did ;)

[sigmavirus24]

I think using the dollar and thus requiring the "version" line ends with a newline is a bad idea : it might end with a whitespace or a comment, it's valid.

No one asked for you to add anything other than the ^.

Thanks for fixing this up. Any further feedback @Lukasa?

[Lukasa]

Nope. Fine by me. =)

[deronnax]

The

Please fix the expression to check for line beginnings and endings

misled me :)

[sigmavirus24]

I misspoke twice then! Sorry for the confusion @deronnax. Thanks for the contribution! 🍰

[deronnax]

You're welcome :)