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

Format python files with black #828

Closed
wants to merge 2 commits into from
Closed

Format python files with black #828

wants to merge 2 commits into from

Conversation

sudoforge
Copy link
Contributor

No description provided.

@sudoforge
Copy link
Contributor Author

@trygveaa per your comment, here's a PR adding black (and formatting all current files). This is done in two separate commits.

trygveaa added a commit that referenced this pull request Mar 20, 2021
Thanks to @sudoforge in #828 for this.
@trygveaa trygveaa closed this in eca1a5a Mar 20, 2021
@trygveaa
Copy link
Member

Thanks! Since the checks in this PR were failing, I ran black myself and committed to master. Turns out the code hit a bug in black, so we had to run it twice, see psf/black#2055.

@sudoforge
Copy link
Contributor Author

Oof, sorry, I should have added a comment here but got caught up in other things. There actually appears to be an error on subsequent formatting (with python < 3), which is why the checks are failing.

I did run with --fast in order to format wee_slack.py in the commit present in this PR. That wasn't the issue here.

@sudoforge
Copy link
Contributor Author

🤷‍♂️ it appears to work for you now, at least on HEAD... if you find spurious CI failures on the lint check, I'd recommend removing it until you hear back on the issue you opened and have a solution.

@trygveaa
Copy link
Member

How do you figure the issue is with python < 3? The checks were failing for all python versions.

As I said, I had to run black twice, because running with black --fast made a file that still wasn't formatted properly (which is why we had to use --fast). You can see there's a diff between this PR and what I committed. The bug in black is with the particular formatting we used, so after formatting it twice it's now formatted properly and works correctly.

@sudoforge sudoforge deleted the format-python-files-with-black branch March 20, 2021 13:33
@sudoforge
Copy link
Contributor Author

sudoforge commented Mar 20, 2021

When running locally on this tree, I am not able to recreate your experience with 3.9.2:

(wee-slack) gh::sudoforge/wee-slack [ 2e42e72 ]
➜ git rev-parse HEAD
2e42e72c96aa2f5dc5b51df50598324c12f2a1a9

^-- this commit is _before_ any files have been formatted (it is the commit that adds black to the deps)


(wee-slack) gh::sudoforge/wee-slack [ 2e42e72 ]
➜ python -V
Python 3.9.2


(wee-slack) gh::sudoforge/wee-slack [ 2e42e72 ]
➜ black --fast wee_slack.py
reformatted wee_slack.py
All done! ✨ 🍰 ✨
1 file reformatted.


(wee-slack) gh::sudoforge/wee-slack [ 2e42e72 ~1 ]
➜ black .
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_everything.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_eventrouter.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_formatting.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_presencechange.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_processsubteamcreated.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_command_reply.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_processreply.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_processteamjoin.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_processsubteamupdated.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_sendmessage.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_process_message.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_linkifytext.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_thread.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/generate_docs.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/generate_weemoji.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_topic_command.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/conftest.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_utf8_helpers.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_unfurl.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_formatted_name.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_unwrap_attachments.py
All done! ✨ 🍰 ✨
21 files reformatted, 2 files left unchanged.


(wee-slack) gh::sudoforge/wee-slack [ 2e42e72 ~22 ]
➜ git commit -am 'all files have now been formatted for the first time'
[detached HEAD e5bafdd] all files have now been formatted for the first time
 22 files changed, 3237 insertions(+), 1955 deletions(-)
 rewrite _pytest/test_formatted_name.py (94%)
 rewrite _pytest/test_linkifytext.py (67%)
 rewrite _pytest/test_unfurl.py (95%)
 rewrite _pytest/test_unwrap_attachments.py (98%)


(wee-slack) gh::sudoforge/wee-slack [ e5bafdd ]
➜ black .
All done! ✨ 🍰 ✨
23 files left unchanged.

That's where my statement comes from. Perhaps I'm incorrect, or missing something obvious. In any case, I'm glad you got it sorted out!

@sudoforge
Copy link
Contributor Author

sudoforge commented Mar 20, 2021

And just to show that it had nothing to do with the order of operations in my previous comment:

(wee-slack) gh::sudoforge/wee-slack [ 2e42e72 ]
➜ black .
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_eventrouter.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_formatting.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_everything.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_command_reply.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_processreply.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_presencechange.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_processsubteamcreated.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_processteamjoin.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_sendmessage.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_processsubteamupdated.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_process_message.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_thread.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_linkifytext.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_topic_command.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/generate_docs.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/generate_weemoji.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/conftest.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_utf8_helpers.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_formatted_name.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_unfurl.py
reformatted /home/sudoforge/code/github.com/sudoforge/wee-slack/_pytest/test_unwrap_attachments.py
error: cannot format /home/sudoforge/code/github.com/sudoforge/wee-slack/wee_slack.py: INTERNAL ERROR: Black produced different code on the second pass of the formatter.  Please report a bug on https://github.com/psf/black/issues.  This diff might be helpful: /tmp/blk_90y12noh.log
Oh no! 💥 💔 💥
21 files reformatted, 1 file left unchanged, 1 file failed to reformat.
black .  7.72s user 0.17s system 137% cpu 5.733 total


(wee-slack) gh::sudoforge/wee-slack [ 2e42e72 ~21 ]
➜ black --fast wee_slack.py
reformatted wee_slack.py
All done! ✨ 🍰 ✨
1 file reformatted.


(wee-slack) gh::sudoforge/wee-slack [ 2e42e72 ~22 ]
➜ git commit -am 'all files have now been formatted for the first time'
[detached HEAD df90991] all files have now been formatted for the first time
 22 files changed, 3237 insertions(+), 1955 deletions(-)
 rewrite _pytest/test_formatted_name.py (94%)
 rewrite _pytest/test_linkifytext.py (67%)
 rewrite _pytest/test_unfurl.py (95%)
 rewrite _pytest/test_unwrap_attachments.py (98%)


(wee-slack) gh::sudoforge/wee-slack [ df90991 ]
➜ black .
All done! ✨ 🍰 ✨
23 files left unchanged.

@trygveaa
Copy link
Member

I should have mentioned this, but you need to touch or otherwise change/save the wee_slack.py file before you run black again. If not, black won't do anything when you run it again. I guess it has a cache of the last run time it checks against mtime or something.

@sudoforge
Copy link
Contributor Author

Ah, that's what I was missing, then. Well, glad you got it sorted out -- I'd been deferring digging in to this until I had more time in the next couple of weeks.

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.

None yet

3 participants