Skip to content

[Python] Added a new python_format.py script to the utils directory we can use to automatically check the formatting for all the Python sources in the project. #29701

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

Merged
merged 1 commit into from
Feb 7, 2020

Conversation

Rostepher
Copy link
Contributor

This script is intentionally written in Python 3 as the black tool does not work with Python 2.7. I don't expect we will start using this script yet, but I wanted to add it for future use.

@Rostepher
Copy link
Contributor Author

@swift-ci please smoke test

"-m",
"black",
"--target-version",
"py27",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now the majority of our sources should remain Python 2.7 compatible. In the future we can update this to py37.

…e can use to automatically check the formatting for all the Python sources in the project.
@Rostepher Rostepher force-pushed the python-format-script branch from c8a6108 to 08225c0 Compare February 7, 2020 09:08
@Rostepher
Copy link
Contributor Author

@swift-ci please smoke test

@Rostepher Rostepher merged commit 167fea2 into swiftlang:master Feb 7, 2020
@Rostepher Rostepher deleted the python-format-script branch February 7, 2020 18:20
@palimondo
Copy link
Contributor

palimondo commented Mar 3, 2020

@Rostepher Thanks for the reply in #26462. I have read this PR since it was mentioned in #29719, but perhaps my point wasn’t articulated well enough.

I understand the goal of any automated formatting tool is to unify coding style on the whole project and to minimize avoidable conflict over personal preferences. We already follow pythonic best practices with python_lint.py and enforce these during smoke test on each PR.

From the quick inspection of Black, it seems the point of that particular formatter is to have 0 configuration options. I find that philosophy rather extreme, since it denies programmers any choice to express one’s thoughts in a readable way to fellow humans. Instead it offers just blind enforcement of rigid rules. As can be seen in #29719, the end effect is actually lower source code readability, since it often turns carefully crafted concise expressions into exploded multi line monstrosities.

Given that we already use python_lint.py to unify Python coding style across the project, what is the actual issue on our project that using Black will solve?

Personally, I don’t see a good reason to disrupt development by mass source code changes, replacing single quote with a double quote everywhere, to get a less readable source code in the end. Did I miss any discussion on Swift forums that lead to this being the project’s consensus going forward? CC @eeckstein @gottesmm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants