Hi Daniel, Great point about accessibility and people with visual impairment. It is important to make sure that the gem5 community is welcoming to all!
Personally, I don't think that 79 -> 88 characters will make too much of a difference, but there's no clear cut off. Increasing the line length is becoming important in python as we move to adding type annotations. This can cause function definitions to grow significantly with well-named types. One more data point to throw out, Google's C++ standard (which is our baseline) says 80 characters for C++. PS: Going back and looking at the history of this conversation in the community, in 2015 we bumped the line length from 78 to 79. It was 78 so the two diff characters would still have an 80 character line. I'm not certain why we went with 79 instead of 80... Cheers, Jason On Thu, Jun 24, 2021 at 10:05 AM Daniel Carvalho via gem5-dev < gem5-dev@gem5.org> wrote: > Hello, > > My two cents: the number of columns is more important for readability and > accessibility than for file size. The higher the number of columns, the > more likely that users with visual impairment will have trouble finding an > efficient-yet-comfortable setup. > > For example, on the one hand, my main gem5 clone's src/ has 92MB, while > the whole dir uses 18GB - i.e., file sizes are negligible compared to their > object counterparts, and AFAIK, changing the number of columns doesn't > affect the size of these files. > > On the other hand, the laptop I'm using to type this has a tiny 14" > screen, and is running at a high resolution (1920x1080). While using > Monospace Regular 14 I am only able to see 166 columns (or 2x82 columns > side by side) in gedit. Increasing the number of columns would require a > reduction of the font size to be able to keep working with 2 windows. While > I am physically able to do this change, some people may not. For reference, > the max font size at which one can work (with my laptop) with a single > window would have to go down from 29 to 25. > > As I said, this is my tiny on-the-go setup. Others may have better setups, > and those with visual impairment will likely invest more to have larger > screens, or use different resolutions, so it may not affect them that much. > Yet, I think this is a much stronger point to be considered than file size. > > Regarding having more columns, how readable are long lines? Are they good > coding practice, or would it be better to split them into multiple > single-line statements? > > Finally, I'd also rather have consistency; if this number changes, it'd be > better to do so for all file extensions, not only Python files. > > That said, I'm against this change as is due to the discrepancy between > advantages and disadvantages. > > Regards, > Daniel > Em quarta-feira, 23 de junho de 2021 18:00:22 BRT, Bobby Bruce via > gem5-dev <gem5-dev@gem5.org> escreveu: > > > I attempted to take some initiative with this relation chain: > https://gem5-review.googlesource.com/c/public/gem5/+/47000/ > > In short, I applied Python Black with a line length of 79, then added a > Python black check to our pre-submit (Kokoro) tests. > > This patchset raised a question as to what line length we really want for > Python. Our official policy has been <=79 characters but Black defaults to > 88 (The black developers argue this default produces a good trade off > between file length and line length). This is a good chance to have this > discussion, so the floor is open. Personally, I'm not massively opposed but > a consistent 79 character limit across the project has worked for us thus > far and I don't personally see much reason to change. > > Kind regards, > Bobby > -- > Dr. Bobby R. Bruce > Room 3050, > Kemper Hall, UC Davis > Davis, > CA, 95616 > > web: https://www.bobbybruce.net > > > On Thu, Mar 4, 2021 at 8:05 AM Giacomo Travaglini via gem5-dev < > gem5-dev@gem5.org> wrote: > > > > > -----Original Message----- > > From: Jason Lowe-Power <ja...@lowepower.com> > > Sent: 04 March 2021 15:27 > > To: gem5 Developer List <gem5-dev@gem5.org> > > Cc: Gabe Black <gabe.bl...@gmail.com>; Giacomo Travaglini > > <giacomo.travagl...@arm.com> > > Subject: Re: [gem5-dev] Re: RFC: run python Black on gem5 python code > > > > Hi all, > > > > > > Some responses inline below. The other thing I'll mention is that I > believe we > > can integrate it in a way where it's completely invisible to most > developers. > > You can edit a file with whatever style you want, then after you run > black it will > > be in the PEP8 style. From my experience, the only things that need to be > > manually changed are quoted strings that are more than 80 characters. > > > > I would be very careful about hiding this to the developers at the point > of automatically changing their code > without having them noticing it. > > I understand you want developers to commit without the hassle of > explicitly dealing with style errors. > On the other hand though we are already doing this for C++ so I don't see > a reason why we shouldn't do the same > for the python world. > > Other options could be to run the linter as a pre-push hook, or to run the > linter as part of our presubmit Jenkins script. > > > On Thu, Mar 4, 2021 at 6:49 AM Giacomo Travaglini via gem5-dev <gem5- > > d...@gem5.org <mailto:gem5-dev@gem5.org> > wrote: > > > > > > Hi Jason, > > > > This is great news. When you say it is not configurable, do you mean it > > is not possible to suppress warning/errors or do you > > mean it is not possible to come up with custom rules? > > > > > > > > There is a way to suppress changes by modifying the file with a comment > `# > > fmt: off`. You can also exclude files with a dotfile (like .gitignore). > > > > > > > > As I presume it's the first case, I am wondering if a different linter > > might provide such capabilities: > > > > https://books.agiliq.com/projects/essential-python- > > tools/en/latest/linters.html > > > > Is there a reason to prefer black over other linters? > > > > > > > > It's owned by the python software foundation, which makes it seem > "official". > > Though the real reason is that it's the first one I looked at and the > only one I > > investigated. > > I actually thought pycodestyle was the "official" linter (I am not sure if > the "official" word makes completely sense though) > > I was asking this because for example in pycodestyle (and prob other > linters as well) there are error codes > https://pycodestyle.pycqa.org/en/latest/intro.html#error-codes you could > use include or exclude lists of errors (to ignore specific types of > warnings). > > Kind regards > > Giacomo > > > > > > > > > Kind regards > > > > Giacomo > > > > > -----Original Message----- > > > From: Gabe Black via gem5-dev <gem5-dev@gem5.org > > <mailto:gem5-dev@gem5.org> > > > > Sent: 04 March 2021 03:59 > > > To: gem5 Developer List <gem5-dev@gem5.org <mailto:gem5- > > d...@gem5.org> > > > > Cc: Gabe Black <gabe.bl...@gmail.com > > <mailto:gabe.bl...@gmail.com> > > > > Subject: [gem5-dev] Re: RFC: run python Black on gem5 python code > > > > > > I'm a little worried about the no exceptions part of that, since we > > might have > > > some weird restrictions that we have to do weird things to work > > around, but I > > > can't really think of an example of that off hand. I'd want to look at > it > > to see > > > how much wiggle room there is in the style, since I think ironclad > > rules which > > > make no accommodation for occasional common sense are maybe > > more > > > trouble than they're worth. I'm not opposed to having at least some > > stated > > > standard for python though, and the "official" one seems like a > > pretty > > > reasonable choice. I guess it's fine with me, up until it causes me > > some sort of > > > problem :-) > > > > > > > > I actually think it's a feature not a bug. With an "uncompromising > Python code > > formatter" there can be no arguments in code review about the style. > > > > > > > > > > Maybe the right thing to do would be to give it a shot but not make > > it > > > compulsory until we have a feeling for how much trouble it is. > > > > > > > > > Gabe > > > > > > On Wed, Mar 3, 2021 at 11:24 AM Bobby Bruce via gem5-dev <gem5- > > > d...@gem5.org <mailto:d...@gem5.org> <mailto:gem5- > > d...@gem5.org <mailto:gem5-dev@gem5.org> > > wrote: > > > > > > > > > Sounds like a good idea to me. > > > --- > > > > > > Dr. Bobby R. Bruce > > > Room 2235, > > > Kemper Hall, UC Davis > > > Davis, > > > CA, 95616 > > > > > > > > > web: https://www.bobbybruce.net > > > > > > > > > > > > On Wed, Mar 3, 2021 at 10:11 AM Daniel Carvalho via gem5-dev > > > <gem5-dev@gem5.org <mailto:gem5-dev@gem5.org> > > <mailto:gem5-dev@gem5.org <mailto:gem5-dev@gem5.org> > > wrote: > > > > > > > > > +1 > > > > > > > > > Em quarta-feira, 3 de março de 2021 14:35:57 BRT, Jason > > > Lowe-Power via gem5-dev <gem5-dev@gem5.org <mailto:gem5- > > d...@gem5.org> <mailto:gem5- <mailto:gem5-> > > > d...@gem5.org <mailto:d...@gem5.org> > > escreveu: > > > > > > > > > Hi all, > > > > > > Right now, we don't have an official style guide for python. > > > Our style guide > > > > > (http://www.gem5.org/documentation/general_docs/development/coding_st > > > yle/) is very C++ focused. > > > > > > I would like to propose adopting a relatively strict PEP 8 style > > > guide: https://www.python.org/dev/peps/pep-0008. This is the > > "official" style > > > guide for python (as much as there is anything official). I say > > "relatively strict" > > > to mean that we will limit our exceptions *as much as possible*. > > > > > > To implement this, Andreas S. recently pointed me to the > > > "Black" package (https://pypi.org/project/black/) which automatically > > fixes > > > code style. I just tried it out with gem5art (patch coming soon) and > > found that > > > it works really well. The only downside is that it's not configurable > at > > all. > > > Adding special cases would be almost impossible. > > > > > > Concrete and specific proposal: > > > - Adopt PEP 8 as our official style guide > > > - Use black to reformat all python code in src/ > > > - Use black to reformat code in configs/ > > > - Use black to reformat other python code > > > > > > - Use black as part of our commit hook to make sure all future > > > python is formatted to PEP 8 > > > > > > Let me know what you think! > > > > > > Cheers, > > > Jason > > > > > > _______________________________________________ > > > gem5-dev mailing list -- gem5-dev@gem5.org <mailto:gem5- > > d...@gem5.org> <mailto:gem5- <mailto:gem5-> > > > d...@gem5.org <mailto:d...@gem5.org> > > > > To unsubscribe send an email to gem5-dev-le...@gem5.org > > <mailto:gem5-dev-le...@gem5.org> > > > <mailto:gem5-dev-le...@gem5.org <mailto:gem5-dev- > > le...@gem5.org> > > > > %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s > > > _______________________________________________ > > > gem5-dev mailing list -- gem5-dev@gem5.org <mailto:gem5- > > d...@gem5.org> <mailto:gem5- <mailto:gem5-> > > > d...@gem5.org <mailto:d...@gem5.org> > > > > To unsubscribe send an email to gem5-dev-le...@gem5.org > > <mailto:gem5-dev-le...@gem5.org> > > > <mailto:gem5-dev-le...@gem5.org <mailto:gem5-dev- > > le...@gem5.org> > > > > %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s > > > > > > _______________________________________________ > > > gem5-dev mailing list -- gem5-dev@gem5.org <mailto:gem5- > > d...@gem5.org> <mailto:gem5- <mailto:gem5-> > > > d...@gem5.org <mailto:d...@gem5.org> > > > > To unsubscribe send an email to gem5-dev-le...@gem5.org > > <mailto:gem5-dev-le...@gem5.org> > > > <mailto:gem5-dev-le...@gem5.org <mailto:gem5-dev- > > le...@gem5.org> > > > > %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s > > > > IMPORTANT NOTICE: The contents of this email and any attachments > > are confidential and may also be privileged. If you are not the intended > > recipient, please notify the sender immediately and do not disclose the > > contents to any other person, use it for any purpose, or store or copy > the > > information in any medium. Thank you. > > _______________________________________________ > > gem5-dev mailing list -- gem5-dev@gem5.org <mailto:gem5- > > d...@gem5.org> > > To unsubscribe send an email to gem5-dev-le...@gem5.org > > <mailto:gem5-dev-le...@gem5.org> > > %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s > > IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended > recipient, please notify the sender immediately and do not disclose the > contents to any other person, use it for any purpose, or store or copy the > information in any medium. Thank you. > _______________________________________________ > gem5-dev mailing list -- gem5-dev@gem5.org > To unsubscribe send an email to gem5-dev-le...@gem5.org > %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s > > _______________________________________________ > gem5-dev mailing list -- gem5-dev@gem5.org > To unsubscribe send an email to gem5-dev-le...@gem5.org > %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s > _______________________________________________ > gem5-dev mailing list -- gem5-dev@gem5.org > To unsubscribe send an email to gem5-dev-le...@gem5.org > %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
_______________________________________________ gem5-dev mailing list -- gem5-dev@gem5.org To unsubscribe send an email to gem5-dev-le...@gem5.org %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
