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

refactor support/interface.py with black #1027

Merged
merged 3 commits into from
Jan 19, 2024

Conversation

rieder
Copy link
Member

@rieder rieder commented Jan 19, 2024

No description provided.

@rieder rieder requested a review from a team as a code owner January 19, 2024 10:46
@LourensVeen
Copy link
Collaborator

Looks black! 😄

I also agree that 88 characters is much better than 80. I've recently been switching my other code bases and it's really noticeable.

@rieder
Copy link
Member Author

rieder commented Jan 19, 2024

Looks black! 😄

I also agree that 88 characters is much better than 80. I've recently been switching my other code bases and it's really noticeable.

Black has 88 as the default, I switched it to 80 in the second commit since I noticed errors with this in my editor.
But I think you're right, 88 looks better.

@LourensVeen
Copy link
Collaborator

Oh, I misread, I thought you'd gone from 80 to 88!

I think the rationale behind 88 is that most code in larger programs is in classes, and so you always lose 8 characters to the indentation. In my experience, those 8 extra characters surprisingly often mean that something just fits on a line as opposed to having to be split, and it makes everything looks better.

I bet your editor has a line length setting somewhere 😄

@rieder
Copy link
Member Author

rieder commented Jan 19, 2024

Yeah, I can see why 88 makes more sense. I'll adjust the value in my editor and revert the second commit.

@rieder rieder merged commit 9e425e8 into amusecode:main Jan 19, 2024
6 of 8 checks passed
@rieder rieder deleted the refactor/support branch January 19, 2024 11:49
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.

2 participants