-
-
Notifications
You must be signed in to change notification settings - Fork 529
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.
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
Remove usage of py.process #622
Conversation
ah, need to update the mock too! |
Codecov Report
@@ Coverage Diff @@
## master #622 +/- ##
==========================================
+ Coverage 93.69% 93.84% +0.15%
==========================================
Files 11 11
Lines 2362 2357 -5
==========================================
- Hits 2213 2212 -1
+ Misses 149 145 -4
Continue to review full report at Codecov.
|
This missing coverage in the diff here seems to be a case I don't think is possible any more -- or at least I don't see any evidence of virtualenv being vendored into tox -- should I just remove that branch? |
Good catch. That changeset is from 2013 and had to do with py2.5 support, so this can definitely go. This is also only used in --showconfig, and where is that vendored code supposed to be? It's not in the project, so this must have gone long ago and that check was just overlooked. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one, you make it look so simple :)
stdout=subprocess.PIPE, | ||
) | ||
out, _ = proc.communicate() | ||
versions.append('virtualenv-{0}'.format(out.decode('UTF-8').strip())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally don't like the .format way of string formatting, but as long as no one is forcing me to use it myself I can live with it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interesting! I prefer it over %
formatting 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@obestwalter Maybe https://pyformat.info/ will convince you otherwise? 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@The-Compiler I am afraid not. I don't see any value in using .format for simple string formatting. So for now I will still choose
'some text %s bla bla' % somevar
over
'some text {0} bla bla'.format(somevar)
... until I know better.
I also wish that Guido had engaged his time machine and introduced f-strings right away rather than in 3.6 ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with clever evil hacks, I could probably get this to work in python2.7
# -*- encoding: future_fstrings -*-
thing = 'world'
print(f'hello {thing}')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hah! Oh dear! 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
O.M.G.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@asottile any performance hit compared to format?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They'll be slower that native f string support but this hack will have identical performance to .format (it translates the f string into format calls in fact). Native f strings are only faster because they are implemented directly in byte code (which a backport can't quite do)
Refs #610