Hi All, This post is for discussing the automatic code formatters for C/C++/*.py that are now available. The current options, as I see them, are clang-format (C/C++) and black (*.py, *.pyi, etc). Neither is perfect to my eye, but I think they are good enough at this point and would reduce amount of style nitpicking. *clang-format* There is already a '.clang-format' file in the numpy repository. The main drawbacks that I see are: - no automatic spacing between function definitions, - all operators, including '*' and '/', get spaces around them, - no extra indenting of continued 'if' conditions, - the order of includes is alphabetical within groups. The groups are currently defined in the 'clang-format' file, but there may still be problems with include order that we will need to fix. I think those can be lived with. The results are good enough that I think it can replace the official C coding style for most things. The maximum line length is currently 79 characters. *black* The black formatter is much improved in its latest version and I think good enough to start using. The main drawbacks that I see are: - all operators, including '*' and '/', get spaces around them, - very long strings are not broken into multiple lines, - lists, tuples, and function signatures are either on one line, or broken into multiple lines of one element/argument each, - the formatting of extended logical expressions could be improved to emphasize the priority of 'and' over 'or' operators There is no current configuration in pyproject.toml, the default maximum line length is 88 characters. I note that setting the line length to 79 notably increases the number of line breaks needed. Note that double quotes are now preferred to single quotes. I think that including black configuration in pyproject.toml would be a good idea, the main question is line length, 79 or 88 characters. The same question of line length can also be raised for clang-format. Lines much longer than 88 are characters are not only a problem for terminals, they are also hard to focus on if you have bad eyes, but lines longer than 79 characters also reduce the number of line breaks needed. Thoughts? Chuck
On Sun, Nov 14, 2021 at 6:14 PM Charles R Harris <charlesr.harris@gmail.com> wrote:
Hi All,
This post is for discussing the automatic code formatters for C/C++/*.py that are now available. The current options, as I see them, are clang-format (C/C++) and black (*.py, *.pyi, etc). Neither is perfect to my eye, but I think they are good enough at this point and would reduce amount of style nitpicking.
*clang-format*
There is already a '.clang-format' file in the numpy repository. The main drawbacks that I see are:
- no automatic spacing between function definitions, - all operators, including '*' and '/', get spaces around them, - no extra indenting of continued 'if' conditions, - the order of includes is alphabetical within groups. The groups are currently defined in the 'clang-format' file, but there may still be problems with include order that we will need to fix.
I think those can be lived with. The results are good enough that I think it can replace the official C coding style for most things. The maximum line length is currently 79 characters.
*black*
The black formatter is much improved in its latest version and I think good enough to start using. The main drawbacks that I see are:
- all operators, including '*' and '/', get spaces around them, - very long strings are not broken into multiple lines, - lists, tuples, and function signatures are either on one line, or broken into multiple lines of one element/argument each, - the formatting of extended logical expressions could be improved to emphasize the priority of 'and' over 'or' operators
There is no current configuration in pyproject.toml, the default maximum line length is 88 characters. I note that setting the line length to 79 notably increases the number of line breaks needed. Note that double quotes are now preferred to single quotes.
I think that including black configuration in pyproject.toml would be a good idea, the main question is line length, 79 or 88 characters. The same question of line length can also be raised for clang-format. Lines much longer than 88 are characters are not only a problem for terminals, they are also hard to focus on if you have bad eyes, but lines longer than 79 characters also reduce the number of line breaks needed.
Thoughts?
I'd suggest: (1) let's discuss `clang-format` for C/C++ separately from `black` for Python. (2) to read https://github.com/scipy/scipy/pull/14330 and https://github.com/scipy/scipy/issues/14354 and then not even continue the discussion on `black`. A detailed proposal with an incremental formatter may have a chance here (xref `darker` and our `tools/linter.py`), a "let's just run black" one seems dead in the water given the people and opinions in the linked SciPy PR and issue from a few months ago. Cheers, Ralf
On Sun, Nov 14, 2021 at 11:40 AM Ralf Gommers <ralf.gommers@gmail.com> wrote:
On Sun, Nov 14, 2021 at 6:14 PM Charles R Harris < charlesr.harris@gmail.com> wrote:
Hi All,
This post is for discussing the automatic code formatters for C/C++/*.py that are now available. The current options, as I see them, are clang-format (C/C++) and black (*.py, *.pyi, etc). Neither is perfect to my eye, but I think they are good enough at this point and would reduce amount of style nitpicking.
*clang-format*
There is already a '.clang-format' file in the numpy repository. The main drawbacks that I see are:
- no automatic spacing between function definitions, - all operators, including '*' and '/', get spaces around them, - no extra indenting of continued 'if' conditions, - the order of includes is alphabetical within groups. The groups are currently defined in the 'clang-format' file, but there may still be problems with include order that we will need to fix.
I think those can be lived with. The results are good enough that I think it can replace the official C coding style for most things. The maximum line length is currently 79 characters.
*black*
The black formatter is much improved in its latest version and I think good enough to start using. The main drawbacks that I see are:
- all operators, including '*' and '/', get spaces around them, - very long strings are not broken into multiple lines, - lists, tuples, and function signatures are either on one line, or broken into multiple lines of one element/argument each, - the formatting of extended logical expressions could be improved to emphasize the priority of 'and' over 'or' operators
There is no current configuration in pyproject.toml, the default maximum line length is 88 characters. I note that setting the line length to 79 notably increases the number of line breaks needed. Note that double quotes are now preferred to single quotes.
I think that including black configuration in pyproject.toml would be a good idea, the main question is line length, 79 or 88 characters. The same question of line length can also be raised for clang-format. Lines much longer than 88 are characters are not only a problem for terminals, they are also hard to focus on if you have bad eyes, but lines longer than 79 characters also reduce the number of line breaks needed.
Thoughts?
I'd suggest: (1) let's discuss `clang-format` for C/C++ separately from `black` for Python. (2) to read https://github.com/scipy/scipy/pull/14330 and https://github.com/scipy/scipy/issues/14354 and then not even continue the discussion on `black`. A detailed proposal with an incremental formatter may have a chance here (xref `darker` and our `tools/linter.py`), a "let's just run black" one seems dead in the water given the people and opinions in the linked SciPy PR and issue from a few months ago.
"black" is under active development and has relaxed their rigid ideas on formatting. I am not advocating blindly reformatting all the files, but having something in pyproject.toml so that folks can run it on new code as a first pass and fix up places where the result is less than optimal. The main place where that might be inconvenient is the expansion of lists/tuples/signatures into vertical lists, but my impression is that one could get used to that and maybe it is an improvement. I think formatters are the future, it is just a question of when the future arrives. Chuck
in pandas we did a one time conversion to using black (a while ago) for python (and recently added a cython formatter); we do also have automatic c/cpp linters as well we have precommit rules to enforce this (and runs on ci) since then we don’t have discussions about formatting anymore :) would encourage numpy to do the same Jeff
On Nov 14, 2021, at 2:32 PM, Charles R Harris <charlesr.harris@gmail.com> wrote:
On Sun, Nov 14, 2021 at 11:40 AM Ralf Gommers <ralf.gommers@gmail.com> wrote:
On Sun, Nov 14, 2021 at 6:14 PM Charles R Harris <charlesr.harris@gmail.com> wrote: Hi All,
This post is for discussing the automatic code formatters for C/C++/*.py that are now available. The current options, as I see them, are clang-format (C/C++) and black (*.py, *.pyi, etc). Neither is perfect to my eye, but I think they are good enough at this point and would reduce amount of style nitpicking.
clang-format
There is already a '.clang-format' file in the numpy repository. The main drawbacks that I see are: no automatic spacing between function definitions, all operators, including '*' and '/', get spaces around them, no extra indenting of continued 'if' conditions, the order of includes is alphabetical within groups. The groups are currently defined in the 'clang-format' file, but there may still be problems with include order that we will need to fix. I think those can be lived with. The results are good enough that I think it can replace the official C coding style for most things. The maximum line length is currently 79 characters.
black
The black formatter is much improved in its latest version and I think good enough to start using. The main drawbacks that I see are: all operators, including '*' and '/', get spaces around them, very long strings are not broken into multiple lines, lists, tuples, and function signatures are either on one line, or broken into multiple lines of one element/argument each, the formatting of extended logical expressions could be improved to emphasize the priority of 'and' over 'or' operators There is no current configuration in pyproject.toml, the default maximum line length is 88 characters. I note that setting the line length to 79 notably increases the number of line breaks needed. Note that double quotes are now preferred to single quotes.
I think that including black configuration in pyproject.toml would be a good idea, the main question is line length, 79 or 88 characters. The same question of line length can also be raised for clang-format. Lines much longer than 88 are characters are not only a problem for terminals, they are also hard to focus on if you have bad eyes, but lines longer than 79 characters also reduce the number of line breaks needed.
Thoughts?
I'd suggest: (1) let's discuss `clang-format` for C/C++ separately from `black` for Python. (2) to read https://github.com/scipy/scipy/pull/14330 and https://github.com/scipy/scipy/issues/14354 and then not even continue the discussion on `black`. A detailed proposal with an incremental formatter may have a chance here (xref `darker` and our `tools/linter.py`), a "let's just run black" one seems dead in the water given the people and opinions in the linked SciPy PR and issue from a few months ago.
"black" is under active development and has relaxed their rigid ideas on formatting. I am not advocating blindly reformatting all the files, but having something in pyproject.toml so that folks can run it on new code as a first pass and fix up places where the result is less than optimal. The main place where that might be inconvenient is the expansion of lists/tuples/signatures into vertical lists, but my impression is that one could get used to that and maybe it is an improvement. I think formatters are the future, it is just a question of when the future arrives.
Chuck _______________________________________________ NumPy-Discussion mailing list -- numpy-discussion@python.org To unsubscribe send an email to numpy-discussion-leave@python.org https://mail.python.org/mailman3/lists/numpy-discussion.python.org/ Member address: jeffreback@gmail.com
On Sun, 14 Nov 2021 at 19:51, Jeff Reback <jeffreback@gmail.com> wrote:
in pandas we did a one time conversion to using black (a while ago) for python (and recently added a cython formatter); we do also have automatic c/cpp linters as well
we have precommit rules to enforce this (and runs on ci)
since then we don’t have discussions about formatting anymore :)
would encourage numpy to do the same
In SymPy we decided not to use Black for the reasons outlined in the PR and issue linked upthread (it ruins mathematical expressions, reference links for sympy discussion below). There was some discussion but once we had an actual PR that applied Black to a nontrivial part of the codebase the decision was easy to make. Subsequently the contributor pushing Black has been exploring yapf which seems more promising: https://github.com/sympy/sympy/pull/22437 My own view is along the lines of Robert Kern's comments in the scipy issue linked upthread. Proponents of autoformatting claim that it solves a problem but that problem doesn't seem like much of a problem to me: https://github.com/sympy/sympy/issues/17287#issuecomment-942637149 Other References: https://github.com/sympy/sympy/issues/17287 https://github.com/sympy/sympy/pull/22434 https://github.com/psf/black/issues/538#issuecomment-962448644 -- Oscar
On Sun, Nov 14, 2021, at 09:13, Charles R Harris wrote:
The black formatter is much improved in its latest version and I think good enough to start using. The main drawbacks that I see are: * all operators, including '*' and '/', get spaces around them, * very long strings are not broken into multiple lines, * lists, tuples, and function signatures are either on one line, or broken into multiple lines of one element/argument each, * the formatting of extended logical expressions could be improved to emphasize the priority of 'and' over 'or' operators
We've also been having a conversation around mathematical formatting here: https://discuss.scientific-python.org/t/how-to-format-mathematical-expressio... I tried yapf recently, and was pleased with the output. One concern about yapf was that it has many configuration options: but the only important thing is that you fix the knobs, then you simply have a different version of black. In my experience, while none of these tools are perfect, not having to have discussions around formatting is completely worth it! Stéfan
On 15 Nov 2021, at 8:23 am, Stefan van der Walt <stefanv@berkeley.edu> wrote:
On Sun, Nov 14, 2021, at 09:13, Charles R Harris wrote:
The black formatter is much improved in its latest version and I think good enough to start using. The main drawbacks that I see are: all operators, including '*' and '/', get spaces around them, very long strings are not broken into multiple lines, lists, tuples, and function signatures are either on one line, or broken into multiple lines of one element/argument each, the formatting of extended logical expressions could be improved to emphasize the priority of 'and' over 'or' operators
We've also been having a conversation around mathematical formatting here: https://discuss.scientific-python.org/t/how-to-format-mathematical-expressio... <https://discuss.scientific-python.org/t/how-to-format-mathematical-expressions/62/8>
I tried yapf recently, and was pleased with the output. One concern about yapf was that it has many configuration options: but the only important thing is that you fix the knobs, then you simply have a different version of black.
In my experience, while none of these tools are perfect, not having to have discussions around formatting is completely worth it!
+1 on everything Stéfan said. I never liked black’s formatting, but I have *absolutely* appreciated having zero discussions/push commits/code suggestions to deal with formatting in napari. I have since added yapf to my own repos with a config I like *and* added yapf auto-formatting-on-save to my VSCode, and I don’t even have to have formatting discussions with *myself* anymore. 😂 It’s very liberating! For reference, here’s my yapf config: https://github.com/jni/skan/blob/74507344b4cd4453cc43b4dbd0b5742fc08eb5a0/.s... As Stéfan said, fix the knobs (yours might be different), then forget about it! Oh, and yes, yapf does allow formatting only the diff. I agree that reformatting the entire code base is problematic. Juan.
I've also used uncrustify for c/c++. Of course this plays hell with revision control. On Sun, Nov 14, 2021 at 6:29 PM Juan Nunez-Iglesias <jni@fastmail.com> wrote:
On 15 Nov 2021, at 8:23 am, Stefan van der Walt <stefanv@berkeley.edu> wrote:
On Sun, Nov 14, 2021, at 09:13, Charles R Harris wrote:
The black formatter is much improved in its latest version and I think good enough to start using. The main drawbacks that I see are:
all operators, including '*' and '/', get spaces around them, very long strings are not broken into multiple lines, lists, tuples, and function signatures are either on one line, or broken into multiple lines of one element/argument each, the formatting of extended logical expressions could be improved to emphasize the priority of 'and' over 'or' operators
We've also been having a conversation around mathematical formatting here: https://discuss.scientific-python.org/t/how-to-format-mathematical-expressio...
I tried yapf recently, and was pleased with the output. One concern about yapf was that it has many configuration options: but the only important thing is that you fix the knobs, then you simply have a different version of black.
In my experience, while none of these tools are perfect, not having to have discussions around formatting is completely worth it!
+1 on everything Stéfan said. I never liked black’s formatting, but I have *absolutely* appreciated having zero discussions/push commits/code suggestions to deal with formatting in napari. I have since added yapf to my own repos with a config I like *and* added yapf auto-formatting-on-save to my VSCode, and I don’t even have to have formatting discussions with *myself* anymore. 😂 It’s very liberating!
For reference, here’s my yapf config:
https://github.com/jni/skan/blob/74507344b4cd4453cc43b4dbd0b5742fc08eb5a0/.s...
As Stéfan said, fix the knobs (yours might be different), then forget about it!
Oh, and yes, yapf does allow formatting only the diff. I agree that reformatting the entire code base is problematic.
Juan. _______________________________________________ NumPy-Discussion mailing list -- numpy-discussion@python.org To unsubscribe send an email to numpy-discussion-leave@python.org https://mail.python.org/mailman3/lists/numpy-discussion.python.org/ Member address: ndbecker2@gmail.com
-- Those who don't understand recursion are doomed to repeat it
On Sun, Nov 14, 2021, at 15:26, Juan Nunez-Iglesias wrote:
Oh, and yes, yapf does allow formatting only the diff. I agree that reformatting the entire code base is problematic.
That's a good point: it's possible to make these changes incrementally. There's something called yapf-diff. It's in the main repo, but not integrated into the pre-commit hook yet. Best regards, Stéfan
On Sun, Nov 14, 2021 at 12:37 PM Ralf Gommers <ralf.gommers@gmail.com> wrote:
On Sun, Nov 14, 2021 at 6:14 PM Charles R Harris < charlesr.harris@gmail.com> wrote:
Hi All,
This post is for discussing the automatic code formatters for C/C++/*.py that are now available. The current options, as I see them, are clang-format (C/C++) and black (*.py, *.pyi, etc). Neither is perfect to my eye, but I think they are good enough at this point and would reduce amount of style nitpicking.
*clang-format*
There is already a '.clang-format' file in the numpy repository. The main drawbacks that I see are:
- no automatic spacing between function definitions, - all operators, including '*' and '/', get spaces around them, - no extra indenting of continued 'if' conditions, - the order of includes is alphabetical within groups. The groups are currently defined in the 'clang-format' file, but there may still be problems with include order that we will need to fix.
I think those can be lived with. The results are good enough that I think it can replace the official C coding style for most things. The maximum line length is currently 79 characters.
*black*
The black formatter is much improved in its latest version and I think good enough to start using. The main drawbacks that I see are:
- all operators, including '*' and '/', get spaces around them, - very long strings are not broken into multiple lines, - lists, tuples, and function signatures are either on one line, or broken into multiple lines of one element/argument each, - the formatting of extended logical expressions could be improved to emphasize the priority of 'and' over 'or' operators
There is no current configuration in pyproject.toml, the default maximum line length is 88 characters. I note that setting the line length to 79 notably increases the number of line breaks needed. Note that double quotes are now preferred to single quotes.
I think that including black configuration in pyproject.toml would be a good idea, the main question is line length, 79 or 88 characters. The same question of line length can also be raised for clang-format. Lines much longer than 88 are characters are not only a problem for terminals, they are also hard to focus on if you have bad eyes, but lines longer than 79 characters also reduce the number of line breaks needed.
Thoughts?
I'd suggest: (1) let's discuss `clang-format` for C/C++ separately from `black` for Python. (2) to read https://github.com/scipy/scipy/pull/14330 and https://github.com/scipy/scipy/issues/14354 and then not even continue the discussion on `black`. A detailed proposal with an incremental formatter may have a chance here (xref `darker` and our `tools/linter.py`), a "let's just run black" one seems dead in the water given the people and opinions in the linked SciPy PR and issue from a few months ago.
I've found this wrapper around clang-format useful for making it behave more like black, in terms of running on a directory of files and producing nice diffs of changes: https://github.com/Sarcasm/run-clang-format It is a single MIT-licensed python file that could be vendored easily.
On Sun, Nov 14, 2021 at 4:28 PM Juan Nunez-Iglesias <jni@fastmail.com> wrote:
On 15 Nov 2021, at 8:23 am, Stefan van der Walt <stefanv@berkeley.edu> wrote:
On Sun, Nov 14, 2021, at 09:13, Charles R Harris wrote:
The black formatter is much improved in its latest version and I think good enough to start using. The main drawbacks that I see are:
- all operators, including '*' and '/', get spaces around them, - very long strings are not broken into multiple lines, - lists, tuples, and function signatures are either on one line, or broken into multiple lines of one element/argument each, - the formatting of extended logical expressions could be improved to emphasize the priority of 'and' over 'or' operators
We've also been having a conversation around mathematical formatting here: https://discuss.scientific-python.org/t/how-to-format-mathematical-expressio...
I tried yapf recently, and was pleased with the output. One concern about yapf was that it has many configuration options: but the only important thing is that you fix the knobs, then you simply have a different version of black.
In my experience, while none of these tools are perfect, not having to have discussions around formatting is completely worth it!
+1 on everything Stéfan said. I never liked black’s formatting, but I have *absolutely* appreciated having zero discussions/push commits/code suggestions to deal with formatting in napari. I have since added yapf to my own repos with a config I like *and* added yapf auto-formatting-on-save to my VSCode, and I don’t even have to have formatting discussions with *myself* anymore. 😂 It’s very liberating!
For reference, here’s my yapf config:
https://github.com/jni/skan/blob/74507344b4cd4453cc43b4dbd0b5742fc08eb5a0/.s...
As Stéfan said, fix the knobs (yours might be different), then forget about it!
Oh, and yes, yapf does allow formatting only the diff. I agree that reformatting the entire code base is problematic.
yapf does look like a better alternative than black. Chuck
On Mon, 2021-11-15 at 14:28 -0700, Charles R Harris wrote:
On Sun, Nov 14, 2021 at 4:28 PM Juan Nunez-Iglesias <jni@fastmail.com> wrote:
<snip>
https://github.com/jni/skan/blob/74507344b4cd4453cc43b4dbd0b5742fc08eb5a0/.s...
As Stéfan said, fix the knobs (yours might be different), then forget about it!
Oh, and yes, yapf does allow formatting only the diff. I agree that reformatting the entire code base is problematic.
yapf does look like a better alternative than black.
I think we could give try yapf/clang-format a shot. Frankly, I would be very happy to just defer most/all of the knob setting and making the call on adoption to you Chuck :). (Unless maybe anyone aims for cross-project sharing of these already.) In the end, I expect we will all quickly get used to the vast majority of changes. And projects that adopted auto-formatters seem happy... clang-format has a couple of knobs we could play around with, e.g.: AlignAfterOpenBracket: DontAlign helps with the `if` issue (but comes at additional indentation costs). And there are the penalty options, e.g.: PenaltyBreakString: 150 # random value which, if set, seem to allow a few characters beyond the 79 limit if it saves a line. There are some places where, IMO, existing line breaks make more sense than automatic breaks: if (npy_parse_arguments("astype", args, len_args, kwnames, "dtype", &PyArray_DescrConverter, &dtype, "|order", &PyArray_OrderConverter, &order, "|casting", &PyArray_CastingConverter, &casting, "|subok", &PyArray_PythonPyIntFromInt, &subok, "|copy", &PyArray_PythonPyIntFromInt, &forcecopy, NULL, NULL, NULL) < 0) { where each parameter starts on its own line, and: static struct PyMethodDef array_module_methods[] = { {"_get_implementing_args", (PyCFunction)array__get_implementing_args, METH_VARARGS, NULL}, {"_get_ndarray_c_version", (PyCFunction)array__get_ndarray_c_version, METH_VARARGS|METH_KEYWORDS, NULL}, ... (Less clear, but in that case I really like the uniformity of having the name on its own line.) But I guess we could add `\\` to force line breaks, or disable formatting for those method defs. Cheers, Sebastian
Chuck _______________________________________________ NumPy-Discussion mailing list -- numpy-discussion@python.org To unsubscribe send an email to numpy-discussion-leave@python.org https://mail.python.org/mailman3/lists/numpy-discussion.python.org/ Member address: sebastian@sipsolutions.net
On Mon, Nov 15, 2021 at 3:02 PM Sebastian Berg <sebastian@sipsolutions.net> wrote:
On Mon, 2021-11-15 at 14:28 -0700, Charles R Harris wrote:
On Sun, Nov 14, 2021 at 4:28 PM Juan Nunez-Iglesias <jni@fastmail.com> wrote:
<snip>
https://github.com/jni/skan/blob/74507344b4cd4453cc43b4dbd0b5742fc08eb5a0/.s...
As Stéfan said, fix the knobs (yours might be different), then forget about it!
Oh, and yes, yapf does allow formatting only the diff. I agree that reformatting the entire code base is problematic.
yapf does look like a better alternative than black.
I think we could give try yapf/clang-format a shot. Frankly, I would be very happy to just defer most/all of the knob setting and making the call on adoption to you Chuck :). (Unless maybe anyone aims for cross-project sharing of these already.)
Thanks :)
In the end, I expect we will all quickly get used to the vast majority of changes. And projects that adopted auto-formatters seem happy...
clang-format has a couple of knobs we could play around with, e.g.:
AlignAfterOpenBracket: DontAlign
helps with the `if` issue (but comes at additional indentation costs). And there are the penalty options, e.g.:
PenaltyBreakString: 150 # random value
which, if set, seem to allow a few characters beyond the 79 limit if it saves a line.
I was hoping someone else would tweak it as we gain experience. I don't think any of the formatting options should be blessed until we gain some experience. At some point it will become "good enough".
There are some places where, IMO, existing line breaks make more sense than automatic breaks:
if (npy_parse_arguments("astype", args, len_args, kwnames, "dtype", &PyArray_DescrConverter, &dtype, "|order", &PyArray_OrderConverter, &order, "|casting", &PyArray_CastingConverter, &casting, "|subok", &PyArray_PythonPyIntFromInt, &subok, "|copy", &PyArray_PythonPyIntFromInt, &forcecopy, NULL, NULL, NULL) < 0) {
where each parameter starts on its own line, and:
static struct PyMethodDef array_module_methods[] = { {"_get_implementing_args", (PyCFunction)array__get_implementing_args, METH_VARARGS, NULL}, {"_get_ndarray_c_version", (PyCFunction)array__get_ndarray_c_version, METH_VARARGS|METH_KEYWORDS, NULL}, ...
(Less clear, but in that case I really like the uniformity of having the name on its own line.)
But I guess we could add `\\` to force line breaks, or disable formatting for those method defs.
Chuck
This read by the Author of Black may be helpful. https://lukasz.langa.pl/36380f86-6d28-4a55-962e-91c2c959db7a/ Whether you use Black or a configured YAPF, or .... I think the discussion of "all in one go" is worth thinking about. -CHB On Mon, Nov 15, 2021 at 2:33 PM Charles R Harris <charlesr.harris@gmail.com> wrote:
On Mon, Nov 15, 2021 at 3:02 PM Sebastian Berg <sebastian@sipsolutions.net> wrote:
On Mon, 2021-11-15 at 14:28 -0700, Charles R Harris wrote:
On Sun, Nov 14, 2021 at 4:28 PM Juan Nunez-Iglesias <jni@fastmail.com> wrote:
<snip>
https://github.com/jni/skan/blob/74507344b4cd4453cc43b4dbd0b5742fc08eb5a0/.s...
As Stéfan said, fix the knobs (yours might be different), then forget about it!
Oh, and yes, yapf does allow formatting only the diff. I agree that reformatting the entire code base is problematic.
yapf does look like a better alternative than black.
I think we could give try yapf/clang-format a shot. Frankly, I would be very happy to just defer most/all of the knob setting and making the call on adoption to you Chuck :). (Unless maybe anyone aims for cross-project sharing of these already.)
Thanks :)
In the end, I expect we will all quickly get used to the vast majority of changes. And projects that adopted auto-formatters seem happy...
clang-format has a couple of knobs we could play around with, e.g.:
AlignAfterOpenBracket: DontAlign
helps with the `if` issue (but comes at additional indentation costs). And there are the penalty options, e.g.:
PenaltyBreakString: 150 # random value
which, if set, seem to allow a few characters beyond the 79 limit if it saves a line.
I was hoping someone else would tweak it as we gain experience. I don't think any of the formatting options should be blessed until we gain some experience. At some point it will become "good enough".
There are some places where, IMO, existing line breaks make more sense than automatic breaks:
if (npy_parse_arguments("astype", args, len_args, kwnames, "dtype", &PyArray_DescrConverter, &dtype, "|order", &PyArray_OrderConverter, &order, "|casting", &PyArray_CastingConverter, &casting, "|subok", &PyArray_PythonPyIntFromInt, &subok, "|copy", &PyArray_PythonPyIntFromInt, &forcecopy, NULL, NULL, NULL) < 0) {
where each parameter starts on its own line, and:
static struct PyMethodDef array_module_methods[] = { {"_get_implementing_args", (PyCFunction)array__get_implementing_args, METH_VARARGS, NULL}, {"_get_ndarray_c_version", (PyCFunction)array__get_ndarray_c_version, METH_VARARGS|METH_KEYWORDS, NULL}, ...
(Less clear, but in that case I really like the uniformity of having the name on its own line.)
But I guess we could add `\\` to force line breaks, or disable formatting for those method defs.
Chuck _______________________________________________ NumPy-Discussion mailing list -- numpy-discussion@python.org To unsubscribe send an email to numpy-discussion-leave@python.org https://mail.python.org/mailman3/lists/numpy-discussion.python.org/ Member address: chris.barker@noaa.gov
-- Christopher Barker, Ph.D. Oceanographer Emergency Response Division NOAA/NOS/OR&R (206) 526-6959 voice 7600 Sand Point Way NE (206) 526-6329 fax Seattle, WA 98115 (206) 526-6317 main reception Chris.Barker@noaa.gov
On Thu, 2021-11-18 at 09:41 -0800, Chris Barker wrote:
This read by the Author of Black may be helpful.
https://lukasz.langa.pl/36380f86-6d28-4a55-962e-91c2c959db7a/
Whether you use Black or a configured YAPF, or .... I think the discussion of "all in one go" is worth thinking about.
Personally, I would be OK with the "all in one go". At the very least it will help us get used to whatever style we got faster. Git can even mark commits as "only style fixups" I think to hide them from `git blame` and following history has never bothered me too much; the only thing that seems tricky there is when functionality is moved between files. The main downside to me is probably that it will creating conflicts in all open PRs. With some coordination/announcement that is maybe OK. Cheers, Sebastian
-CHB
On Mon, Nov 15, 2021 at 2:33 PM Charles R Harris <charlesr.harris@gmail.com> wrote:
On Mon, Nov 15, 2021 at 3:02 PM Sebastian Berg <sebastian@sipsolutions.net> wrote:
On Mon, 2021-11-15 at 14:28 -0700, Charles R Harris wrote:
On Sun, Nov 14, 2021 at 4:28 PM Juan Nunez-Iglesias <jni@fastmail.com> wrote:
<snip>
https://github.com/jni/skan/blob/74507344b4cd4453cc43b4dbd0b5742fc08eb5a0/.s...
As Stéfan said, fix the knobs (yours might be different), then forget about it!
Oh, and yes, yapf does allow formatting only the diff. I agree that reformatting the entire code base is problematic.
yapf does look like a better alternative than black.
I think we could give try yapf/clang-format a shot. Frankly, I would be very happy to just defer most/all of the knob setting and making the call on adoption to you Chuck :). (Unless maybe anyone aims for cross-project sharing of these already.)
Thanks :)
In the end, I expect we will all quickly get used to the vast majority of changes. And projects that adopted auto-formatters seem happy...
clang-format has a couple of knobs we could play around with, e.g.:
AlignAfterOpenBracket: DontAlign
helps with the `if` issue (but comes at additional indentation costs). And there are the penalty options, e.g.:
PenaltyBreakString: 150 # random value
which, if set, seem to allow a few characters beyond the 79 limit if it saves a line.
I was hoping someone else would tweak it as we gain experience. I don't think any of the formatting options should be blessed until we gain some experience. At some point it will become "good enough".
There are some places where, IMO, existing line breaks make more sense than automatic breaks:
if (npy_parse_arguments("astype", args, len_args, kwnames, "dtype", &PyArray_DescrConverter, &dtype, "|order", &PyArray_OrderConverter, &order, "|casting", &PyArray_CastingConverter, &casting, "|subok", &PyArray_PythonPyIntFromInt, &subok, "|copy", &PyArray_PythonPyIntFromInt, &forcecopy, NULL, NULL, NULL) < 0) {
where each parameter starts on its own line, and:
static struct PyMethodDef array_module_methods[] = { {"_get_implementing_args", (PyCFunction)array__get_implementing_args, METH_VARARGS, NULL}, {"_get_ndarray_c_version", (PyCFunction)array__get_ndarray_c_version, METH_VARARGS|METH_KEYWORDS, NULL}, ...
(Less clear, but in that case I really like the uniformity of having the name on its own line.)
But I guess we could add `\\` to force line breaks, or disable formatting for those method defs.
Chuck _______________________________________________ NumPy-Discussion mailing list -- numpy-discussion@python.org To unsubscribe send an email to numpy-discussion-leave@python.org https://mail.python.org/mailman3/lists/numpy-discussion.python.org/ Member address: chris.barker@noaa.gov
_______________________________________________ NumPy-Discussion mailing list -- numpy-discussion@python.org To unsubscribe send an email to numpy-discussion-leave@python.org https://mail.python.org/mailman3/lists/numpy-discussion.python.org/ Member address: sebastian@sipsolutions.net
On Thu, Nov 18, 2021, at 09:51, Sebastian Berg wrote:
Git can even mark commits as "only style fixups" I think to hide them from `git blame` and following history has never bothered me too much; the only thing that seems tricky there is when functionality is moved between files. The main downside to me is probably that it will creating conflicts in all open PRs. With some coordination/announcement that is maybe OK.
if we do this, we should probably go through each of the 200+ open PRs (or, at least, the non-conflicted ones), apply the formatter, and then squash the PR into a single commit. We can do that by script. Stéfan
On 18/11/2021 19:07, Stefan van der Walt wrote:
if we do this, we should probably go through each of the 200+ open PRs (or, at least, the non-conflicted ones), apply the formatter, and then squash the PR into a single commit. We can do that by script.
We had to deal with this issue in scikit-learn as well, and you might find the guide on resolving such conflicts in https://github.com/scikit-learn/scikit-learn/issues/20301 helpful. -- Roman
I think some of the pain points raised here regarding massive churn on existing PRs & conflicts would be addressed by what Ralf said a few emails ago:
A detailed proposal with an incremental formatter may have a chance here (xref `darker` and our `tools/linter.py`), a "let's just run black" one seems dead in the water given the people and opinions in the linked SciPy PR and issue from a few months ago.
Why not focusing energies on this incremental approach? I think all folks want to (1) end discussions about code style, (2) avoid weird formatting on math expressions (that black doesn't seem to handle very well) and (3) avoid "breaking the world". Regardless of the specific formatter (black, blue, yapf w/ tweaks), doing it incrementally only on code touched by new PRs would at least provide a less scary way forward. Juan Luis On November 22, 2021, Roman Yurchak <rth.yurchak@gmail.com> wrote:
if we do this, we should probably go through each of the 200+ open PRs (or, at least, the non-conflicted ones), apply the formatter, and
On 18/11/2021 19:07, Stefan van der Walt wrote: then squash the PR into a single commit. We can do that by script.
We had to deal with this issue in scikit-learn as well, and you might find the guide on resolving such conflicts in https://github.com/scikit-learn/scikit-learn/issues/20301 helpful.
-- Roman _______________________________________________ NumPy-Discussion mailing list -- numpy-discussion@python.org To unsubscribe send an email to numpy-discussion-leave@python.org https://mail.python.org/mailman3/lists/numpy-discussion.python.org/ Member address: hello@juanlu.space
Is there a way to figure out which files are not touched by any open PR? That way numpy might be able to do a lot more than an incremental code alignment.
This discussion and the linked gist may be of some help: https://github.com/scikit-learn/scikit-learn/issues/11336 On Mon, Nov 22, 2021 at 12:02 PM Andrew Nelson <andyfaff@gmail.com> wrote:
Is there a way to figure out which files are not touched by any open PR? That way numpy might be able to do a lot more than an incremental code alignment. _______________________________________________ NumPy-Discussion mailing list -- numpy-discussion@python.org To unsubscribe send an email to numpy-discussion-leave@python.org https://mail.python.org/mailman3/lists/numpy-discussion.python.org/ Member address: adrin.jalali@gmail.com
participants (14)
-
Adrin -
Andrew Nelson -
Charles R Harris -
Chris Barker -
Jeff Reback -
Juan Luis Cano Rodríguez -
Juan Nunez-Iglesias -
Neal Becker -
Oscar Benjamin -
Ralf Gommers -
Roman Yurchak -
Sebastian Berg -
Stanley Seibert -
Stefan van der Walt