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.

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

Update %-format call to use str.format #6973

Closed
chrahunt opened this issue Sep 4, 2019 · 17 comments · Fixed by #7178, #7826 or #7840
Closed

Update %-format call to use str.format #6973

chrahunt opened this issue Sep 4, 2019 · 17 comments · Fixed by #7178, #7826 or #7840
Labels
auto-locked Outdated issues that have been locked by automation state: awaiting PR Feature discussed, PR is needed type: maintenance Related to Development and Maintenance Processes

Comments

@chrahunt
Copy link
Member

chrahunt commented Sep 4, 2019

What's the problem this feature will solve?

Currently in src/pip/_internal/operations/prepare.py, we use %-style string formatting.

Describe the solution you'd like

We should use the .format method for consistency with the rest of the code base.

We should also consider doing the same for the rest of the code base. Whoever takes care of this specific issue should make a general issue for that!

Posted in its original form by @pradyunsg in #6968 (comment)


This issue is a good starting point for anyone who wants to help out with pip's development -- it's simple and the process of fixing this should be a good introduction to pip's development workflow.

@triage-new-issues triage-new-issues bot added the S: needs triage Issues/PRs that need to be triaged label Sep 4, 2019
@chrahunt chrahunt added good first issue A good item for first time contributors to work on state: awaiting PR Feature discussed, PR is needed type: maintenance Related to Development and Maintenance Processes labels Sep 4, 2019
@triage-new-issues triage-new-issues bot removed S: needs triage Issues/PRs that need to be triaged labels Sep 4, 2019
@aixtools
Copy link

aixtools commented Sep 4, 2019

I like simple.

As long as you are not expecting this yesterday, I'll be happy to take this.

@dyspop
Copy link
Contributor

dyspop commented Sep 5, 2019

👋 submitting PR now

@dyspop
Copy link
Contributor

dyspop commented Sep 5, 2019

is there a general issue? happy to do that too

@aixtools
Copy link

aixtools commented Sep 5, 2019

All yours then @dyspop

@atugushev
Copy link
Contributor

FYI, to catch %-style string formatting in the code consider flake8-printf-formatting plugin for flake8.

Usage:

$ pip install flake8 flake8-printf-formatting
$ flake8 src/pip/_internal/operations/prepare.py --select=MOD
src/pip/_internal/operations/prepare.py:111:13: MOD001 do not use printf-style string formatting
src/pip/_internal/operations/prepare.py:147:21: MOD001 do not use printf-style string formatting
src/pip/_internal/operations/prepare.py:217:21: MOD001 do not use printf-style string formatting
src/pip/_internal/operations/prepare.py:249:21: MOD001 do not use printf-style string formatting
src/pip/_internal/operations/prepare.py:277:13: MOD001 do not use printf-style string formatting

following the #6763 (comment)
/cc @pradyunsg

@cjerdonek
Copy link
Member

One clarification / caveat: we probably want to exclude logging calls from this change. I can provide reasons if desired.

@pradyunsg
Copy link
Member

@cjerdonek I agree -- #6968 (comment).

I do wonder if there's some way to make the format strings for logging calls to use the new-style formatting. Let's discuss in a new issue.

@pfmoore
Copy link
Member

pfmoore commented Sep 12, 2019

I do wonder if there's some way to make the format strings for logging calls to use the new-style formatting

The style parameter documented here allows this. But it's Python 3 only. Can we stop supporting Python 2 now, please? 🙂

@pradyunsg
Copy link
Member

@pfmoore I wish. :(

I guess we could write a Formatter that would do this handling for us. It shouldn't be too difficult. Anyway, I opened #7009 for discussing this further.

@pradyunsg
Copy link
Member

Reopened since there's still more references of this.

@alexisdurieux
Copy link

Are there any reasons not to use f-strings ?

@atugushev
Copy link
Contributor

atugushev commented Nov 12, 2019

f-strings are not compatible with python 2.7 and 3.5.

@deepak1725
Copy link
Contributor

Working on it.

@jaraco
Copy link
Member

jaraco commented Feb 29, 2020

I've done some work on this issue in the bugfix/6973-format-method branch. In that patch, I've also patched addressed two regressions introduced in the earlier work. I'm taking a break because this translation is a lot of work.

@jaraco jaraco removed the good first issue A good item for first time contributors to work on label Mar 6, 2020
@jaraco
Copy link
Member

jaraco commented Mar 6, 2020

I've removed the good first issue label. Someone coming new to this issue has a lot of history to contend with.

@pradyunsg
Copy link
Member

Thanks everyone who's worked on this! ^>^

@atugushev
Copy link
Contributor

atugushev commented Mar 10, 2020

Could be related to this issue. I see now these logs on pip install ...:

Collecting pytest==3.8.2
  Downloading pytest-3.8.2-py2.py3-none-any.whl (209 kB)
     |████████████████████████████████| {downloaded} {download_speed} {pretty_eta}

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Apr 15, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Apr 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation state: awaiting PR Feature discussed, PR is needed type: maintenance Related to Development and Maintenance Processes
Projects
None yet
10 participants