Skip to content

CLN: 29547 replace .format() with f-strings #32044

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

Conversation

smartvinnetou
Copy link
Contributor

@smartvinnetou smartvinnetou commented Feb 16, 2020

This PR replaces .format() with f-strings as requested in #29547.

It did this replacement in scripts/find_commits_touching_func.py

  • tests added / passed
  • passes black pandas
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff

@smartvinnetou smartvinnetou changed the title replaced .format() with f-strings CLN: 29547 replace .format() with f-strings Feb 16, 2020
@smartvinnetou smartvinnetou marked this pull request as ready for review February 16, 2020 23:13
Copy link
Member

@datapythonista datapythonista left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One comment, other than that looks good.

f"{h[:HASH_LEN]:{HASH_LEN}} "
f"{d.isoformat()[:10]:10} "
f"{s:<{SUBJ_LEN}} "
f"{p[-20:]:<{PATH_LEN}}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is changing the behavior, can you test this and make sure it works as the original.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In what way do you see the behaviour changed?

@datapythonista
Copy link
Member

I was checking, and seems like this script is not working, and I don't think anyone is using it. I'm removing it in #32071, so better to close this PR. Thanks for the work anyway, and sorry for not detecting that earlier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Code Style Code style, linting, code_checks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants