-
Notifications
You must be signed in to change notification settings - Fork 74
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
Switch from 88 character lines back to 80 #177
Comments
👋 Thanks for opening your first issue here! Please make sure you filled out the template with as much detail as possible. You might also want to take a look at our Contributing Guide and Code of Conduct. |
I am not a verde developer, just a follower of the project. In my opinion, 80 is way too limiting. No modern terminal is limited to 80 characters. On my own macbook with my default/preferred font size I am able to have a terminal window take up half my 15" screen with 100 characters: For all of my projects I use 120 characters, but try to keep docstrings to 80-100. I personally don't think 100 is too much to ask for. |
On ArviZ / PyStan3 we use |
Fair point @djhoese, but in my opinion, very excluding. You assume that everyone has big screens with high resolution. Although that becomes more the standard, it still isn't reality for everyone in the world. Having a terminal and an editor side by side with a bigger width than 80 is still not possible in many cases. More importantly, and as far as I know, 80 is the standard in every terminal and at least some editors. So by using the defaults setting of black you force your users to adjust their terminal. Just for your package. I don't buy that. But again, that is my personal opinion. I thought I mention it, but feel free to simply close it @leouieda, if you don't agree. (I wanted to open an issue on the black-GitHub page. But they have already many heated discussion about it there. I think it is good that everyone has the freedom to choose whatever they want. But having the default at 88 is a crime, in my opinion. Someone might use the defaults not even realizing she/he is severely limiting the experience for a part of its user base, and break longstanding standards.) |
(I also forgot to mentioned visually impaired [having myself not the best sight] and elderly folks, who will thank you very much if they have not to set a higher resolution to accommodate more characters per line.) |
Yes, you are right, sorry. I was mostly addressing your statement that most terminals default to 80. That has not been my experience, but maybe that's because I usually immediately grab the corner and make the window bigger. I also agree that in some cases like docstrings, 80 characters are easier to read. It has been my experience that in some coding situations having a hard limit of 80 can make the code uglier and harder to read. In general these are special cases though. |
I agree. Specifically, if you follow the suggestions of having However, in many cases it also makes code more readable if long statements are broken-up into more lines. |
@prisae thanks for sharing your experience 👍 I personally never have a terminal that's not full screen so the 88 limit was always fine to me. But you have a good point, particularly regarding the docstrings. I don't particularly mind switching since it's automatic and doesn't really require much work. I'll give it a try to see if how big of a hassle it will be. I'll keep flake8 and pylint set to 88, though. All in all, I'd rather have you to hack away at the code happily than adhere to a standard. Well, this particular standard :) |
OK, the code changes aren't that bad and I'd be happy to switch. The problem is that black still doesn't format docstrings or comments. So all of the docstrings and comments will have to manually wrapped at 80 char. It's not too hard in Vim but still requires opening all of the files one by one because the RST identation is not always respected by vim. This is something I'm looking forward to doing right now. It's not just the code, there is also all of the examples and tutorials. I'll leave this open for now in case someone wants to take a stab at it. Or for a time when I'm tired and need to do something mindless :) |
max-line-length
to 80
Another concern is: If we do this now, will someone come up later and complain that we need to switch to 100 char? I really don't like going back and forth and chose black specifically so we wouldn't have to think about it. @prisae is there any way that we could make this better for you without having to touch the whole code base? |
I read more yesterday on the black issues, and it seems to be a very heated and opinionated discussion, ranging from 80 to 100, 120, and more. I did not intended to start a flame war. I just looked up the docs of a functionality of I agree in that it is primarily important for docstrings. If the odd line of code is longer than 80 it doesn't disturb the experience that much. So I close this issue, and I apologize for the tempest in a tea pot. If you want to take away anything from this discussion @leouieda, then I suggest to limit docstrings to 80 characters. But from now on, hence every new code or if you change something in existing code. I agree with you that I wouldn't go back and change all the files just for the sake of changing it. Enjoy the Sunday everyone! |
I constantly struggle with these two sides of myself :)
No need to apologize. It's great that you brought this to my attention and I'll definitely keep it in mind when starting new projects. I hand't even considered that this would be a problem.
That is a sensible thing to do. From the black repo, it seems like they are working on this. When the implement it, we can take another look at converting the repo to 80 chars again. |
FYI @prisae this conversation made me rethink my own strategies for some of the libraries I maintain. I'm thinking about switching to 80 characters for docstrings as it is more likely that people will be looking at documentation on smaller screens. We (again, not a verde maintainer) will prefer 80 characters for code but will allow up to 120 characters if it makes sense (it rarely does). Thanks for bringing this to my attention as well. |
Docstrings with 88 characters (the Black line length) don't show up nicely in Jupyter and IPython (since the display is 80 chars). This means that our docstrings were unreadable. See fatiando/verde#177 fatiando/community#9 fatiando/community#10 for context. Make flake8 check that docstrings (and comments, sadly) are 79 characters so we can make sure we don't do it again accidentally. Have to check comments as well because the flake8 flag doesn't discriminate between them.
Docstrings with 88 characters (the Black line length) don't show up nicely in Jupyter and IPython (since the display is 80 chars). This means that our docstrings were unreadable. See fatiando/verde#177 fatiando/community#9 fatiando/community#10 for context. Make flake8 check that docstrings (and comments, sadly) are 79 characters so we can make sure we don't do it again accidentally. Have to check comments as well because the flake8 flag doesn't discriminate between them.
Docstrings with 88 characters (the Black line length) don't show up nicely in Jupyter and IPython (since the display is 80 chars). This means that our docstrings were unreadable. See fatiando/verde#177 fatiando/community#9 fatiando/community#10 for context. Make flake8 check that docstrings (and comments, sadly) are 79 characters so we can make sure we don't do it again accidentally. Have to check comments as well because the flake8 flag doesn't discriminate between them. Fixes #121
As much as I like
black
, and as much as I understand its advantages, its default maximum line length drives me crazy. The maximum of 80 is there for a good reason, as terminals usually have this limit. So if edit in a traditional editor, or look at the docs or the code in a terminal, a longer line-length makes working very difficult. And this is not that uncommon, think, e.g., about working on a server without GUI. I guess this is a plea that not everyone is working in a Notebook or with Atom, Sublime, VSCode etc.I suggest to restrict the
max-line-length
to 80. As far as I could see, there are three places currently with amax-line-length
:I attach here two screenshots, one from GVim and one from an IPython console.
The examples are for documentation, but the same applies for the code.
The text was updated successfully, but these errors were encountered: