You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@arrow.apache.org by Antoine Pitrou <an...@python.org> on 2020/04/02 13:30:15 UTC
[Python] black vs. autopep8
Hello,
I've put up two PRs to compare the effect of running black vs. autopep8
on the Python codebase.
* black: https://github.com/apache/arrow/pull/6810
65 files changed, 7855 insertions(+), 5215 deletions(-)
* autopep8: https://github.com/apache/arrow/pull/6811
20 files changed, 137 insertions(+), 118 deletions(-)
I've configured black to try and minimize changes (for example, avoid
normalizing string quoting style). Still, the number of changes is
humongous and they add 2600 lines to the codebase (which is a tangible
amount of vertical space).
Regards
Antoine.
Re: [Python] black vs. autopep8
Posted by Rok Mihevc <ro...@gmail.com>.
+1 for autopep8
On Thu, Apr 9, 2020 at 4:45 PM Wes McKinney <we...@gmail.com> wrote:
> So to summarize, it seems that what we are agreeing in this thread is
> to not debate readability about otherwise PEP8-compliant Python code
> in code reviews, is that right? Absent a consensus about a change, the
> outcome is basically "no change". The addition of autopep8 as a tool
> for automated PEP8 fixes (as opposed to manual ones, which is what's
> required right now) seems reasonable in that it does not make invasive
> changes to the current codebase.
>
> We can also abandon this discussion and not use any tool. Either way
> it would be good to reach a conclusion and move on.
>
> - Wes
>
> On Thu, Apr 9, 2020 at 3:05 AM Uwe L. Korn <uw...@xhochy.com> wrote:
> >
> > The non-configurability of black is one of the strongest arguments I see
> for black. The codestyle will always be subjective. From previous
> discussions I know that my personal preference of readability conflicts
> with that of Antoine and Wes, so will probably others. We have the same
> issue with using the Google C++ style guide in C++. Not everyone agrees
> with it but at least it is a guide that is adopted widely (and thus seen in
> other projects quite often) and has well outlined reasonings for most of
> its choices.
> >
> > On Wed, Apr 8, 2020, at 10:25 PM, Xinbin Huang wrote:
> > > Another option that we can look into is yapf (
> > > https://github.com/google/yapf). It is similar to black but more
> tweakable.
> > > Also, it is recently adopted by the Apache Beam project. PR is here
> > > https://github.com/apache/beam/pull/10684/files
> > >
> > > Bin
> > >
> > > On Wed, Apr 8, 2020 at 1:18 PM Wes McKinney <we...@gmail.com>
> wrote:
> > >
> > > > I don't think it's possible unfortunately. From the README: "Black
> > > > reformats entire files in place. It is not configurable."
> > > >
> > > > The main concern about Black is the impact that it has on
> readability.
> > > > I share this concern as the subjective style choices it makes are
> > > > quite different from the way I've been writing Python code the last
> 12
> > > > years. That doesn't mean I'm right and it's wrong, of course. In this
> > > > project, it doesn't seem like we're losing much energy to people
> > > > reformatting arguments lists or adding line breaks here and there,
> > > > etc. FWIW, from reading the Black docs, it doesn't strike me that the
> > > > tool is designed to maximize readability, but rather to make
> > > > formatting deterministic and reduce code diffs caused by
> reformatting.
> > > >
> > > > My opinion is that we should simply avoid debating code style in code
> > > > reviews as long as the code passes the PEP8 checks. Employing
> autopep8
> > > > is an improvement over the status quo (which is that developers must
> > > > fix flake8 / PEP8 warnings by hand). We can always revisit the Black
> > > > discussion in the future if we find that subjective code formatting
> > > > (beyond PEP8 compliance) is taking up our energy inappropriately.
> > > >
> > > > On Wed, Apr 8, 2020 at 2:13 PM Rok Mihevc <ro...@gmail.com>
> wrote:
> > > > >
> > > > > Could we 'tone down' black to get the desired behavior?
> > > > > I'm ok with either tool.
> > > > >
> > > > > Rok
> > > > >
> > > > > On Wed, Apr 8, 2020 at 8:00 PM Wes McKinney <we...@gmail.com>
> wrote:
> > > > >
> > > > > > On Wed, Apr 8, 2020 at 12:47 PM Neal Richardson
> > > > > > <ne...@gmail.com> wrote:
> > > > > > >
> > > > > > > So autopep8 doesn't fix everything? Sounds inferior to me. That
> > > > said, I'm
> > > > > > > in favor of any resolution that increases our automation of
> this and
> > > > > > > decreases the energy we expend debating it.
> > > > > >
> > > > > > It does fix everything, where "everything" is compliance with
> PEP8,
> > > > > > which I think is the thing we are most interested in.
> > > > > >
> > > > > > Black makes a bunch of other arbitrary (albeit consistent)
> > > > > > reformattings that don't affect PEP8 compliance.
> > > > > >
> > > > > > > Neal
> > > > > > >
> > > > > > >
> > > > > > > On Wed, Apr 8, 2020 at 10:34 AM Wes McKinney <
> wesmckinn@gmail.com>
> > > > > > wrote:
> > > > > > >
> > > > > > > > Circling back on this, it seems there isn't consensus about
> > > > switching
> > > > > > > > to Black, and using autopep8 at least will give us an easy
> way to
> > > > > > > > maintain PEP8 compliance and help contributors fix linting
> failures
> > > > > > > > detected by flake8 (but not all, e.g. unused imports would
> need to
> > > > be
> > > > > > > > manually removed). Would everyone be on board with using
> autopep8?
> > > > > > > >
> > > > > > > > On Thu, Apr 2, 2020 at 9:07 AM Wes McKinney <
> wesmckinn@gmail.com>
> > > > > > wrote:
> > > > > > > > >
> > > > > > > > > I'm personally fine with the Black changes. After the
> one-time
> > > > cost
> > > > > > of
> > > > > > > > > reformatting the codebase, it will take any personal
> preferences
> > > > out
> > > > > > > > > of code formatting (I admit that I have several myself,
> but I
> > > > don't
> > > > > > > > > mind the normalization provided by Black). I hope that
> Cython
> > > > support
> > > > > > > > > comes soon since a great deal of our code is Cython
> > > > > > > > >
> > > > > > > > > On Thu, Apr 2, 2020 at 9:00 AM Jacek Pliszka <
> > > > > > jacek.pliszka@gmail.com>
> > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > Hi!
> > > > > > > > > >
> > > > > > > > > > I believe amount of changes is not that important.
> > > > > > > > > >
> > > > > > > > > > In my opinion, what matters is which format will allow
> > > > reviewers
> > > > > > to be
> > > > > > > > > > more efficient.
> > > > > > > > > >
> > > > > > > > > > The committer can always reformat as they like. It is
> harder
> > > > for
> > > > > > the
> > > > > > > > reviewer.
> > > > > > > > > >
> > > > > > > > > > BR,
> > > > > > > > > >
> > > > > > > > > > Jacek
> > > > > > > > > >
> > > > > > > > > > czw., 2 kwi 2020 o 15:32 Antoine Pitrou <
> antoine@python.org>
> > > > > > > > napisał(a):
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > PS: in both cases, Cython files are not processed.
> autopep8
> > > > is
> > > > > > > > actually
> > > > > > > > > > > able to process them, but the comparison wouldn't be
> > > > > > > > apples-to-apples.
> > > > > > > > > > >
> > > > > > > > > > > (that said, autopep8 gives suboptimal results on Cython
> > > > files,
> > > > > > for
> > > > > > > > > > > example it changes "&c_variable" to "& c_variable" and
> > > > > > > > > > > "void* ptr" to "void * ptr")
> > > > > > > > > > >
> > > > > > > > > > > Regards
> > > > > > > > > > >
> > > > > > > > > > > Antoine.
> > > > > > > > > > >
> > > > > > > > > > > Le 02/04/2020 à 15:30, Antoine Pitrou a écrit :
> > > > > > > > > > > >
> > > > > > > > > > > > Hello,
> > > > > > > > > > > >
> > > > > > > > > > > > I've put up two PRs to compare the effect of running
> black
> > > > vs.
> > > > > > > > autopep8
> > > > > > > > > > > > on the Python codebase.
> > > > > > > > > > > >
> > > > > > > > > > > > * black: https://github.com/apache/arrow/pull/6810
> > > > > > > > > > > > 65 files changed, 7855 insertions(+), 5215
> deletions(-)
> > > > > > > > > > > >
> > > > > > > > > > > > * autopep8:
> https://github.com/apache/arrow/pull/6811
> > > > > > > > > > > > 20 files changed, 137 insertions(+), 118
> deletions(-)
> > > > > > > > > > > >
> > > > > > > > > > > > I've configured black to try and minimize changes
> (for
> > > > example,
> > > > > > > > avoid
> > > > > > > > > > > > normalizing string quoting style). Still, the
> number of
> > > > > > changes is
> > > > > > > > > > > > humongous and they add 2600 lines to the codebase
> (which
> > > > is a
> > > > > > > > tangible
> > > > > > > > > > > > amount of vertical space).
> > > > > > > > > > > >
> > > > > > > > > > > > Regards
> > > > > > > > > > > >
> > > > > > > > > > > > Antoine.
> > > > > > > > > > > >
> > > > > > > >
> > > > > >
> > > >
> > >
>
Re: [Python] black vs. autopep8
Posted by Wes McKinney <we...@gmail.com>.
So to summarize, it seems that what we are agreeing in this thread is
to not debate readability about otherwise PEP8-compliant Python code
in code reviews, is that right? Absent a consensus about a change, the
outcome is basically "no change". The addition of autopep8 as a tool
for automated PEP8 fixes (as opposed to manual ones, which is what's
required right now) seems reasonable in that it does not make invasive
changes to the current codebase.
We can also abandon this discussion and not use any tool. Either way
it would be good to reach a conclusion and move on.
- Wes
On Thu, Apr 9, 2020 at 3:05 AM Uwe L. Korn <uw...@xhochy.com> wrote:
>
> The non-configurability of black is one of the strongest arguments I see for black. The codestyle will always be subjective. From previous discussions I know that my personal preference of readability conflicts with that of Antoine and Wes, so will probably others. We have the same issue with using the Google C++ style guide in C++. Not everyone agrees with it but at least it is a guide that is adopted widely (and thus seen in other projects quite often) and has well outlined reasonings for most of its choices.
>
> On Wed, Apr 8, 2020, at 10:25 PM, Xinbin Huang wrote:
> > Another option that we can look into is yapf (
> > https://github.com/google/yapf). It is similar to black but more tweakable.
> > Also, it is recently adopted by the Apache Beam project. PR is here
> > https://github.com/apache/beam/pull/10684/files
> >
> > Bin
> >
> > On Wed, Apr 8, 2020 at 1:18 PM Wes McKinney <we...@gmail.com> wrote:
> >
> > > I don't think it's possible unfortunately. From the README: "Black
> > > reformats entire files in place. It is not configurable."
> > >
> > > The main concern about Black is the impact that it has on readability.
> > > I share this concern as the subjective style choices it makes are
> > > quite different from the way I've been writing Python code the last 12
> > > years. That doesn't mean I'm right and it's wrong, of course. In this
> > > project, it doesn't seem like we're losing much energy to people
> > > reformatting arguments lists or adding line breaks here and there,
> > > etc. FWIW, from reading the Black docs, it doesn't strike me that the
> > > tool is designed to maximize readability, but rather to make
> > > formatting deterministic and reduce code diffs caused by reformatting.
> > >
> > > My opinion is that we should simply avoid debating code style in code
> > > reviews as long as the code passes the PEP8 checks. Employing autopep8
> > > is an improvement over the status quo (which is that developers must
> > > fix flake8 / PEP8 warnings by hand). We can always revisit the Black
> > > discussion in the future if we find that subjective code formatting
> > > (beyond PEP8 compliance) is taking up our energy inappropriately.
> > >
> > > On Wed, Apr 8, 2020 at 2:13 PM Rok Mihevc <ro...@gmail.com> wrote:
> > > >
> > > > Could we 'tone down' black to get the desired behavior?
> > > > I'm ok with either tool.
> > > >
> > > > Rok
> > > >
> > > > On Wed, Apr 8, 2020 at 8:00 PM Wes McKinney <we...@gmail.com> wrote:
> > > >
> > > > > On Wed, Apr 8, 2020 at 12:47 PM Neal Richardson
> > > > > <ne...@gmail.com> wrote:
> > > > > >
> > > > > > So autopep8 doesn't fix everything? Sounds inferior to me. That
> > > said, I'm
> > > > > > in favor of any resolution that increases our automation of this and
> > > > > > decreases the energy we expend debating it.
> > > > >
> > > > > It does fix everything, where "everything" is compliance with PEP8,
> > > > > which I think is the thing we are most interested in.
> > > > >
> > > > > Black makes a bunch of other arbitrary (albeit consistent)
> > > > > reformattings that don't affect PEP8 compliance.
> > > > >
> > > > > > Neal
> > > > > >
> > > > > >
> > > > > > On Wed, Apr 8, 2020 at 10:34 AM Wes McKinney <we...@gmail.com>
> > > > > wrote:
> > > > > >
> > > > > > > Circling back on this, it seems there isn't consensus about
> > > switching
> > > > > > > to Black, and using autopep8 at least will give us an easy way to
> > > > > > > maintain PEP8 compliance and help contributors fix linting failures
> > > > > > > detected by flake8 (but not all, e.g. unused imports would need to
> > > be
> > > > > > > manually removed). Would everyone be on board with using autopep8?
> > > > > > >
> > > > > > > On Thu, Apr 2, 2020 at 9:07 AM Wes McKinney <we...@gmail.com>
> > > > > wrote:
> > > > > > > >
> > > > > > > > I'm personally fine with the Black changes. After the one-time
> > > cost
> > > > > of
> > > > > > > > reformatting the codebase, it will take any personal preferences
> > > out
> > > > > > > > of code formatting (I admit that I have several myself, but I
> > > don't
> > > > > > > > mind the normalization provided by Black). I hope that Cython
> > > support
> > > > > > > > comes soon since a great deal of our code is Cython
> > > > > > > >
> > > > > > > > On Thu, Apr 2, 2020 at 9:00 AM Jacek Pliszka <
> > > > > jacek.pliszka@gmail.com>
> > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > Hi!
> > > > > > > > >
> > > > > > > > > I believe amount of changes is not that important.
> > > > > > > > >
> > > > > > > > > In my opinion, what matters is which format will allow
> > > reviewers
> > > > > to be
> > > > > > > > > more efficient.
> > > > > > > > >
> > > > > > > > > The committer can always reformat as they like. It is harder
> > > for
> > > > > the
> > > > > > > reviewer.
> > > > > > > > >
> > > > > > > > > BR,
> > > > > > > > >
> > > > > > > > > Jacek
> > > > > > > > >
> > > > > > > > > czw., 2 kwi 2020 o 15:32 Antoine Pitrou <an...@python.org>
> > > > > > > napisał(a):
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > PS: in both cases, Cython files are not processed. autopep8
> > > is
> > > > > > > actually
> > > > > > > > > > able to process them, but the comparison wouldn't be
> > > > > > > apples-to-apples.
> > > > > > > > > >
> > > > > > > > > > (that said, autopep8 gives suboptimal results on Cython
> > > files,
> > > > > for
> > > > > > > > > > example it changes "&c_variable" to "& c_variable" and
> > > > > > > > > > "void* ptr" to "void * ptr")
> > > > > > > > > >
> > > > > > > > > > Regards
> > > > > > > > > >
> > > > > > > > > > Antoine.
> > > > > > > > > >
> > > > > > > > > > Le 02/04/2020 à 15:30, Antoine Pitrou a écrit :
> > > > > > > > > > >
> > > > > > > > > > > Hello,
> > > > > > > > > > >
> > > > > > > > > > > I've put up two PRs to compare the effect of running black
> > > vs.
> > > > > > > autopep8
> > > > > > > > > > > on the Python codebase.
> > > > > > > > > > >
> > > > > > > > > > > * black: https://github.com/apache/arrow/pull/6810
> > > > > > > > > > > 65 files changed, 7855 insertions(+), 5215 deletions(-)
> > > > > > > > > > >
> > > > > > > > > > > * autopep8: https://github.com/apache/arrow/pull/6811
> > > > > > > > > > > 20 files changed, 137 insertions(+), 118 deletions(-)
> > > > > > > > > > >
> > > > > > > > > > > I've configured black to try and minimize changes (for
> > > example,
> > > > > > > avoid
> > > > > > > > > > > normalizing string quoting style). Still, the number of
> > > > > changes is
> > > > > > > > > > > humongous and they add 2600 lines to the codebase (which
> > > is a
> > > > > > > tangible
> > > > > > > > > > > amount of vertical space).
> > > > > > > > > > >
> > > > > > > > > > > Regards
> > > > > > > > > > >
> > > > > > > > > > > Antoine.
> > > > > > > > > > >
> > > > > > >
> > > > >
> > >
> >
Re: [Python] black vs. autopep8
Posted by "Uwe L. Korn" <uw...@xhochy.com>.
The non-configurability of black is one of the strongest arguments I see for black. The codestyle will always be subjective. From previous discussions I know that my personal preference of readability conflicts with that of Antoine and Wes, so will probably others. We have the same issue with using the Google C++ style guide in C++. Not everyone agrees with it but at least it is a guide that is adopted widely (and thus seen in other projects quite often) and has well outlined reasonings for most of its choices.
On Wed, Apr 8, 2020, at 10:25 PM, Xinbin Huang wrote:
> Another option that we can look into is yapf (
> https://github.com/google/yapf). It is similar to black but more tweakable.
> Also, it is recently adopted by the Apache Beam project. PR is here
> https://github.com/apache/beam/pull/10684/files
>
> Bin
>
> On Wed, Apr 8, 2020 at 1:18 PM Wes McKinney <we...@gmail.com> wrote:
>
> > I don't think it's possible unfortunately. From the README: "Black
> > reformats entire files in place. It is not configurable."
> >
> > The main concern about Black is the impact that it has on readability.
> > I share this concern as the subjective style choices it makes are
> > quite different from the way I've been writing Python code the last 12
> > years. That doesn't mean I'm right and it's wrong, of course. In this
> > project, it doesn't seem like we're losing much energy to people
> > reformatting arguments lists or adding line breaks here and there,
> > etc. FWIW, from reading the Black docs, it doesn't strike me that the
> > tool is designed to maximize readability, but rather to make
> > formatting deterministic and reduce code diffs caused by reformatting.
> >
> > My opinion is that we should simply avoid debating code style in code
> > reviews as long as the code passes the PEP8 checks. Employing autopep8
> > is an improvement over the status quo (which is that developers must
> > fix flake8 / PEP8 warnings by hand). We can always revisit the Black
> > discussion in the future if we find that subjective code formatting
> > (beyond PEP8 compliance) is taking up our energy inappropriately.
> >
> > On Wed, Apr 8, 2020 at 2:13 PM Rok Mihevc <ro...@gmail.com> wrote:
> > >
> > > Could we 'tone down' black to get the desired behavior?
> > > I'm ok with either tool.
> > >
> > > Rok
> > >
> > > On Wed, Apr 8, 2020 at 8:00 PM Wes McKinney <we...@gmail.com> wrote:
> > >
> > > > On Wed, Apr 8, 2020 at 12:47 PM Neal Richardson
> > > > <ne...@gmail.com> wrote:
> > > > >
> > > > > So autopep8 doesn't fix everything? Sounds inferior to me. That
> > said, I'm
> > > > > in favor of any resolution that increases our automation of this and
> > > > > decreases the energy we expend debating it.
> > > >
> > > > It does fix everything, where "everything" is compliance with PEP8,
> > > > which I think is the thing we are most interested in.
> > > >
> > > > Black makes a bunch of other arbitrary (albeit consistent)
> > > > reformattings that don't affect PEP8 compliance.
> > > >
> > > > > Neal
> > > > >
> > > > >
> > > > > On Wed, Apr 8, 2020 at 10:34 AM Wes McKinney <we...@gmail.com>
> > > > wrote:
> > > > >
> > > > > > Circling back on this, it seems there isn't consensus about
> > switching
> > > > > > to Black, and using autopep8 at least will give us an easy way to
> > > > > > maintain PEP8 compliance and help contributors fix linting failures
> > > > > > detected by flake8 (but not all, e.g. unused imports would need to
> > be
> > > > > > manually removed). Would everyone be on board with using autopep8?
> > > > > >
> > > > > > On Thu, Apr 2, 2020 at 9:07 AM Wes McKinney <we...@gmail.com>
> > > > wrote:
> > > > > > >
> > > > > > > I'm personally fine with the Black changes. After the one-time
> > cost
> > > > of
> > > > > > > reformatting the codebase, it will take any personal preferences
> > out
> > > > > > > of code formatting (I admit that I have several myself, but I
> > don't
> > > > > > > mind the normalization provided by Black). I hope that Cython
> > support
> > > > > > > comes soon since a great deal of our code is Cython
> > > > > > >
> > > > > > > On Thu, Apr 2, 2020 at 9:00 AM Jacek Pliszka <
> > > > jacek.pliszka@gmail.com>
> > > > > > wrote:
> > > > > > > >
> > > > > > > > Hi!
> > > > > > > >
> > > > > > > > I believe amount of changes is not that important.
> > > > > > > >
> > > > > > > > In my opinion, what matters is which format will allow
> > reviewers
> > > > to be
> > > > > > > > more efficient.
> > > > > > > >
> > > > > > > > The committer can always reformat as they like. It is harder
> > for
> > > > the
> > > > > > reviewer.
> > > > > > > >
> > > > > > > > BR,
> > > > > > > >
> > > > > > > > Jacek
> > > > > > > >
> > > > > > > > czw., 2 kwi 2020 o 15:32 Antoine Pitrou <an...@python.org>
> > > > > > napisał(a):
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > PS: in both cases, Cython files are not processed. autopep8
> > is
> > > > > > actually
> > > > > > > > > able to process them, but the comparison wouldn't be
> > > > > > apples-to-apples.
> > > > > > > > >
> > > > > > > > > (that said, autopep8 gives suboptimal results on Cython
> > files,
> > > > for
> > > > > > > > > example it changes "&c_variable" to "& c_variable" and
> > > > > > > > > "void* ptr" to "void * ptr")
> > > > > > > > >
> > > > > > > > > Regards
> > > > > > > > >
> > > > > > > > > Antoine.
> > > > > > > > >
> > > > > > > > > Le 02/04/2020 à 15:30, Antoine Pitrou a écrit :
> > > > > > > > > >
> > > > > > > > > > Hello,
> > > > > > > > > >
> > > > > > > > > > I've put up two PRs to compare the effect of running black
> > vs.
> > > > > > autopep8
> > > > > > > > > > on the Python codebase.
> > > > > > > > > >
> > > > > > > > > > * black: https://github.com/apache/arrow/pull/6810
> > > > > > > > > > 65 files changed, 7855 insertions(+), 5215 deletions(-)
> > > > > > > > > >
> > > > > > > > > > * autopep8: https://github.com/apache/arrow/pull/6811
> > > > > > > > > > 20 files changed, 137 insertions(+), 118 deletions(-)
> > > > > > > > > >
> > > > > > > > > > I've configured black to try and minimize changes (for
> > example,
> > > > > > avoid
> > > > > > > > > > normalizing string quoting style). Still, the number of
> > > > changes is
> > > > > > > > > > humongous and they add 2600 lines to the codebase (which
> > is a
> > > > > > tangible
> > > > > > > > > > amount of vertical space).
> > > > > > > > > >
> > > > > > > > > > Regards
> > > > > > > > > >
> > > > > > > > > > Antoine.
> > > > > > > > > >
> > > > > >
> > > >
> >
>
Re: [Python] black vs. autopep8
Posted by Xinbin Huang <bi...@gmail.com>.
Another option that we can look into is yapf (
https://github.com/google/yapf). It is similar to black but more tweakable.
Also, it is recently adopted by the Apache Beam project. PR is here
https://github.com/apache/beam/pull/10684/files
Bin
On Wed, Apr 8, 2020 at 1:18 PM Wes McKinney <we...@gmail.com> wrote:
> I don't think it's possible unfortunately. From the README: "Black
> reformats entire files in place. It is not configurable."
>
> The main concern about Black is the impact that it has on readability.
> I share this concern as the subjective style choices it makes are
> quite different from the way I've been writing Python code the last 12
> years. That doesn't mean I'm right and it's wrong, of course. In this
> project, it doesn't seem like we're losing much energy to people
> reformatting arguments lists or adding line breaks here and there,
> etc. FWIW, from reading the Black docs, it doesn't strike me that the
> tool is designed to maximize readability, but rather to make
> formatting deterministic and reduce code diffs caused by reformatting.
>
> My opinion is that we should simply avoid debating code style in code
> reviews as long as the code passes the PEP8 checks. Employing autopep8
> is an improvement over the status quo (which is that developers must
> fix flake8 / PEP8 warnings by hand). We can always revisit the Black
> discussion in the future if we find that subjective code formatting
> (beyond PEP8 compliance) is taking up our energy inappropriately.
>
> On Wed, Apr 8, 2020 at 2:13 PM Rok Mihevc <ro...@gmail.com> wrote:
> >
> > Could we 'tone down' black to get the desired behavior?
> > I'm ok with either tool.
> >
> > Rok
> >
> > On Wed, Apr 8, 2020 at 8:00 PM Wes McKinney <we...@gmail.com> wrote:
> >
> > > On Wed, Apr 8, 2020 at 12:47 PM Neal Richardson
> > > <ne...@gmail.com> wrote:
> > > >
> > > > So autopep8 doesn't fix everything? Sounds inferior to me. That
> said, I'm
> > > > in favor of any resolution that increases our automation of this and
> > > > decreases the energy we expend debating it.
> > >
> > > It does fix everything, where "everything" is compliance with PEP8,
> > > which I think is the thing we are most interested in.
> > >
> > > Black makes a bunch of other arbitrary (albeit consistent)
> > > reformattings that don't affect PEP8 compliance.
> > >
> > > > Neal
> > > >
> > > >
> > > > On Wed, Apr 8, 2020 at 10:34 AM Wes McKinney <we...@gmail.com>
> > > wrote:
> > > >
> > > > > Circling back on this, it seems there isn't consensus about
> switching
> > > > > to Black, and using autopep8 at least will give us an easy way to
> > > > > maintain PEP8 compliance and help contributors fix linting failures
> > > > > detected by flake8 (but not all, e.g. unused imports would need to
> be
> > > > > manually removed). Would everyone be on board with using autopep8?
> > > > >
> > > > > On Thu, Apr 2, 2020 at 9:07 AM Wes McKinney <we...@gmail.com>
> > > wrote:
> > > > > >
> > > > > > I'm personally fine with the Black changes. After the one-time
> cost
> > > of
> > > > > > reformatting the codebase, it will take any personal preferences
> out
> > > > > > of code formatting (I admit that I have several myself, but I
> don't
> > > > > > mind the normalization provided by Black). I hope that Cython
> support
> > > > > > comes soon since a great deal of our code is Cython
> > > > > >
> > > > > > On Thu, Apr 2, 2020 at 9:00 AM Jacek Pliszka <
> > > jacek.pliszka@gmail.com>
> > > > > wrote:
> > > > > > >
> > > > > > > Hi!
> > > > > > >
> > > > > > > I believe amount of changes is not that important.
> > > > > > >
> > > > > > > In my opinion, what matters is which format will allow
> reviewers
> > > to be
> > > > > > > more efficient.
> > > > > > >
> > > > > > > The committer can always reformat as they like. It is harder
> for
> > > the
> > > > > reviewer.
> > > > > > >
> > > > > > > BR,
> > > > > > >
> > > > > > > Jacek
> > > > > > >
> > > > > > > czw., 2 kwi 2020 o 15:32 Antoine Pitrou <an...@python.org>
> > > > > napisał(a):
> > > > > > > >
> > > > > > > >
> > > > > > > > PS: in both cases, Cython files are not processed. autopep8
> is
> > > > > actually
> > > > > > > > able to process them, but the comparison wouldn't be
> > > > > apples-to-apples.
> > > > > > > >
> > > > > > > > (that said, autopep8 gives suboptimal results on Cython
> files,
> > > for
> > > > > > > > example it changes "&c_variable" to "& c_variable" and
> > > > > > > > "void* ptr" to "void * ptr")
> > > > > > > >
> > > > > > > > Regards
> > > > > > > >
> > > > > > > > Antoine.
> > > > > > > >
> > > > > > > > Le 02/04/2020 à 15:30, Antoine Pitrou a écrit :
> > > > > > > > >
> > > > > > > > > Hello,
> > > > > > > > >
> > > > > > > > > I've put up two PRs to compare the effect of running black
> vs.
> > > > > autopep8
> > > > > > > > > on the Python codebase.
> > > > > > > > >
> > > > > > > > > * black: https://github.com/apache/arrow/pull/6810
> > > > > > > > > 65 files changed, 7855 insertions(+), 5215 deletions(-)
> > > > > > > > >
> > > > > > > > > * autopep8: https://github.com/apache/arrow/pull/6811
> > > > > > > > > 20 files changed, 137 insertions(+), 118 deletions(-)
> > > > > > > > >
> > > > > > > > > I've configured black to try and minimize changes (for
> example,
> > > > > avoid
> > > > > > > > > normalizing string quoting style). Still, the number of
> > > changes is
> > > > > > > > > humongous and they add 2600 lines to the codebase (which
> is a
> > > > > tangible
> > > > > > > > > amount of vertical space).
> > > > > > > > >
> > > > > > > > > Regards
> > > > > > > > >
> > > > > > > > > Antoine.
> > > > > > > > >
> > > > >
> > >
>
Re: [Python] black vs. autopep8
Posted by Wes McKinney <we...@gmail.com>.
I don't think it's possible unfortunately. From the README: "Black
reformats entire files in place. It is not configurable."
The main concern about Black is the impact that it has on readability.
I share this concern as the subjective style choices it makes are
quite different from the way I've been writing Python code the last 12
years. That doesn't mean I'm right and it's wrong, of course. In this
project, it doesn't seem like we're losing much energy to people
reformatting arguments lists or adding line breaks here and there,
etc. FWIW, from reading the Black docs, it doesn't strike me that the
tool is designed to maximize readability, but rather to make
formatting deterministic and reduce code diffs caused by reformatting.
My opinion is that we should simply avoid debating code style in code
reviews as long as the code passes the PEP8 checks. Employing autopep8
is an improvement over the status quo (which is that developers must
fix flake8 / PEP8 warnings by hand). We can always revisit the Black
discussion in the future if we find that subjective code formatting
(beyond PEP8 compliance) is taking up our energy inappropriately.
On Wed, Apr 8, 2020 at 2:13 PM Rok Mihevc <ro...@gmail.com> wrote:
>
> Could we 'tone down' black to get the desired behavior?
> I'm ok with either tool.
>
> Rok
>
> On Wed, Apr 8, 2020 at 8:00 PM Wes McKinney <we...@gmail.com> wrote:
>
> > On Wed, Apr 8, 2020 at 12:47 PM Neal Richardson
> > <ne...@gmail.com> wrote:
> > >
> > > So autopep8 doesn't fix everything? Sounds inferior to me. That said, I'm
> > > in favor of any resolution that increases our automation of this and
> > > decreases the energy we expend debating it.
> >
> > It does fix everything, where "everything" is compliance with PEP8,
> > which I think is the thing we are most interested in.
> >
> > Black makes a bunch of other arbitrary (albeit consistent)
> > reformattings that don't affect PEP8 compliance.
> >
> > > Neal
> > >
> > >
> > > On Wed, Apr 8, 2020 at 10:34 AM Wes McKinney <we...@gmail.com>
> > wrote:
> > >
> > > > Circling back on this, it seems there isn't consensus about switching
> > > > to Black, and using autopep8 at least will give us an easy way to
> > > > maintain PEP8 compliance and help contributors fix linting failures
> > > > detected by flake8 (but not all, e.g. unused imports would need to be
> > > > manually removed). Would everyone be on board with using autopep8?
> > > >
> > > > On Thu, Apr 2, 2020 at 9:07 AM Wes McKinney <we...@gmail.com>
> > wrote:
> > > > >
> > > > > I'm personally fine with the Black changes. After the one-time cost
> > of
> > > > > reformatting the codebase, it will take any personal preferences out
> > > > > of code formatting (I admit that I have several myself, but I don't
> > > > > mind the normalization provided by Black). I hope that Cython support
> > > > > comes soon since a great deal of our code is Cython
> > > > >
> > > > > On Thu, Apr 2, 2020 at 9:00 AM Jacek Pliszka <
> > jacek.pliszka@gmail.com>
> > > > wrote:
> > > > > >
> > > > > > Hi!
> > > > > >
> > > > > > I believe amount of changes is not that important.
> > > > > >
> > > > > > In my opinion, what matters is which format will allow reviewers
> > to be
> > > > > > more efficient.
> > > > > >
> > > > > > The committer can always reformat as they like. It is harder for
> > the
> > > > reviewer.
> > > > > >
> > > > > > BR,
> > > > > >
> > > > > > Jacek
> > > > > >
> > > > > > czw., 2 kwi 2020 o 15:32 Antoine Pitrou <an...@python.org>
> > > > napisał(a):
> > > > > > >
> > > > > > >
> > > > > > > PS: in both cases, Cython files are not processed. autopep8 is
> > > > actually
> > > > > > > able to process them, but the comparison wouldn't be
> > > > apples-to-apples.
> > > > > > >
> > > > > > > (that said, autopep8 gives suboptimal results on Cython files,
> > for
> > > > > > > example it changes "&c_variable" to "& c_variable" and
> > > > > > > "void* ptr" to "void * ptr")
> > > > > > >
> > > > > > > Regards
> > > > > > >
> > > > > > > Antoine.
> > > > > > >
> > > > > > > Le 02/04/2020 à 15:30, Antoine Pitrou a écrit :
> > > > > > > >
> > > > > > > > Hello,
> > > > > > > >
> > > > > > > > I've put up two PRs to compare the effect of running black vs.
> > > > autopep8
> > > > > > > > on the Python codebase.
> > > > > > > >
> > > > > > > > * black: https://github.com/apache/arrow/pull/6810
> > > > > > > > 65 files changed, 7855 insertions(+), 5215 deletions(-)
> > > > > > > >
> > > > > > > > * autopep8: https://github.com/apache/arrow/pull/6811
> > > > > > > > 20 files changed, 137 insertions(+), 118 deletions(-)
> > > > > > > >
> > > > > > > > I've configured black to try and minimize changes (for example,
> > > > avoid
> > > > > > > > normalizing string quoting style). Still, the number of
> > changes is
> > > > > > > > humongous and they add 2600 lines to the codebase (which is a
> > > > tangible
> > > > > > > > amount of vertical space).
> > > > > > > >
> > > > > > > > Regards
> > > > > > > >
> > > > > > > > Antoine.
> > > > > > > >
> > > >
> >
Re: [Python] black vs. autopep8
Posted by Rok Mihevc <ro...@gmail.com>.
Could we 'tone down' black to get the desired behavior?
I'm ok with either tool.
Rok
On Wed, Apr 8, 2020 at 8:00 PM Wes McKinney <we...@gmail.com> wrote:
> On Wed, Apr 8, 2020 at 12:47 PM Neal Richardson
> <ne...@gmail.com> wrote:
> >
> > So autopep8 doesn't fix everything? Sounds inferior to me. That said, I'm
> > in favor of any resolution that increases our automation of this and
> > decreases the energy we expend debating it.
>
> It does fix everything, where "everything" is compliance with PEP8,
> which I think is the thing we are most interested in.
>
> Black makes a bunch of other arbitrary (albeit consistent)
> reformattings that don't affect PEP8 compliance.
>
> > Neal
> >
> >
> > On Wed, Apr 8, 2020 at 10:34 AM Wes McKinney <we...@gmail.com>
> wrote:
> >
> > > Circling back on this, it seems there isn't consensus about switching
> > > to Black, and using autopep8 at least will give us an easy way to
> > > maintain PEP8 compliance and help contributors fix linting failures
> > > detected by flake8 (but not all, e.g. unused imports would need to be
> > > manually removed). Would everyone be on board with using autopep8?
> > >
> > > On Thu, Apr 2, 2020 at 9:07 AM Wes McKinney <we...@gmail.com>
> wrote:
> > > >
> > > > I'm personally fine with the Black changes. After the one-time cost
> of
> > > > reformatting the codebase, it will take any personal preferences out
> > > > of code formatting (I admit that I have several myself, but I don't
> > > > mind the normalization provided by Black). I hope that Cython support
> > > > comes soon since a great deal of our code is Cython
> > > >
> > > > On Thu, Apr 2, 2020 at 9:00 AM Jacek Pliszka <
> jacek.pliszka@gmail.com>
> > > wrote:
> > > > >
> > > > > Hi!
> > > > >
> > > > > I believe amount of changes is not that important.
> > > > >
> > > > > In my opinion, what matters is which format will allow reviewers
> to be
> > > > > more efficient.
> > > > >
> > > > > The committer can always reformat as they like. It is harder for
> the
> > > reviewer.
> > > > >
> > > > > BR,
> > > > >
> > > > > Jacek
> > > > >
> > > > > czw., 2 kwi 2020 o 15:32 Antoine Pitrou <an...@python.org>
> > > napisał(a):
> > > > > >
> > > > > >
> > > > > > PS: in both cases, Cython files are not processed. autopep8 is
> > > actually
> > > > > > able to process them, but the comparison wouldn't be
> > > apples-to-apples.
> > > > > >
> > > > > > (that said, autopep8 gives suboptimal results on Cython files,
> for
> > > > > > example it changes "&c_variable" to "& c_variable" and
> > > > > > "void* ptr" to "void * ptr")
> > > > > >
> > > > > > Regards
> > > > > >
> > > > > > Antoine.
> > > > > >
> > > > > > Le 02/04/2020 à 15:30, Antoine Pitrou a écrit :
> > > > > > >
> > > > > > > Hello,
> > > > > > >
> > > > > > > I've put up two PRs to compare the effect of running black vs.
> > > autopep8
> > > > > > > on the Python codebase.
> > > > > > >
> > > > > > > * black: https://github.com/apache/arrow/pull/6810
> > > > > > > 65 files changed, 7855 insertions(+), 5215 deletions(-)
> > > > > > >
> > > > > > > * autopep8: https://github.com/apache/arrow/pull/6811
> > > > > > > 20 files changed, 137 insertions(+), 118 deletions(-)
> > > > > > >
> > > > > > > I've configured black to try and minimize changes (for example,
> > > avoid
> > > > > > > normalizing string quoting style). Still, the number of
> changes is
> > > > > > > humongous and they add 2600 lines to the codebase (which is a
> > > tangible
> > > > > > > amount of vertical space).
> > > > > > >
> > > > > > > Regards
> > > > > > >
> > > > > > > Antoine.
> > > > > > >
> > >
>
Re: [Python] black vs. autopep8
Posted by Micah Kornfield <em...@gmail.com>.
My preference order would be:
"black" then "autopep8" then "no change"
Echoing Joris's and Neal's comments I'm a huge fan of having tools fix
things for me (and the more it handles the better). Maybe I just need to
setup my editor better to deal with PEP8 issue but I find I spend more time
then I would want making the linter happy, so any automation is better then
none.
For reference 2600 lines additions in Black is about 7% addition to the
code base. I find most of these make it easier to read (but I haven't been
doing a lot of python programming lately). The one thing that looks ugly,
but i would guess fade over time is placing closing braces on there own
line in some cases.
Would starting with autopep8 and see if linting is still a pain point, then
reconsider black?
Thanks,
Micah
On Thu, Apr 9, 2020 at 10:36 AM Joris Van den Bossche <
jorisvandenbossche@gmail.com> wrote:
> > > So autopep8 doesn't fix everything? Sounds inferior to me. That said,
> I'm
> > > in favor of any resolution that increases our automation of this and
> > > decreases the energy we expend debating it.
> >
> > It does fix everything, where "everything" is compliance with PEP8,
> > which I think is the thing we are most interested in.
> >
>
> Note that autopep8 does not fix everything, even for PEP8 compliance.
> At least, it can often not automatically fix "too long line length" for
> code lines (that was actually the trigger for me to start the thread about
> adopting black).
>
Re: [Python] black vs. autopep8
Posted by Joris Van den Bossche <jo...@gmail.com>.
> > So autopep8 doesn't fix everything? Sounds inferior to me. That said, I'm
> > in favor of any resolution that increases our automation of this and
> > decreases the energy we expend debating it.
>
> It does fix everything, where "everything" is compliance with PEP8,
> which I think is the thing we are most interested in.
>
Note that autopep8 does not fix everything, even for PEP8 compliance.
At least, it can often not automatically fix "too long line length" for
code lines (that was actually the trigger for me to start the thread about
adopting black).
Re: [Python] black vs. autopep8
Posted by Wes McKinney <we...@gmail.com>.
On Wed, Apr 8, 2020 at 12:47 PM Neal Richardson
<ne...@gmail.com> wrote:
>
> So autopep8 doesn't fix everything? Sounds inferior to me. That said, I'm
> in favor of any resolution that increases our automation of this and
> decreases the energy we expend debating it.
It does fix everything, where "everything" is compliance with PEP8,
which I think is the thing we are most interested in.
Black makes a bunch of other arbitrary (albeit consistent)
reformattings that don't affect PEP8 compliance.
> Neal
>
>
> On Wed, Apr 8, 2020 at 10:34 AM Wes McKinney <we...@gmail.com> wrote:
>
> > Circling back on this, it seems there isn't consensus about switching
> > to Black, and using autopep8 at least will give us an easy way to
> > maintain PEP8 compliance and help contributors fix linting failures
> > detected by flake8 (but not all, e.g. unused imports would need to be
> > manually removed). Would everyone be on board with using autopep8?
> >
> > On Thu, Apr 2, 2020 at 9:07 AM Wes McKinney <we...@gmail.com> wrote:
> > >
> > > I'm personally fine with the Black changes. After the one-time cost of
> > > reformatting the codebase, it will take any personal preferences out
> > > of code formatting (I admit that I have several myself, but I don't
> > > mind the normalization provided by Black). I hope that Cython support
> > > comes soon since a great deal of our code is Cython
> > >
> > > On Thu, Apr 2, 2020 at 9:00 AM Jacek Pliszka <ja...@gmail.com>
> > wrote:
> > > >
> > > > Hi!
> > > >
> > > > I believe amount of changes is not that important.
> > > >
> > > > In my opinion, what matters is which format will allow reviewers to be
> > > > more efficient.
> > > >
> > > > The committer can always reformat as they like. It is harder for the
> > reviewer.
> > > >
> > > > BR,
> > > >
> > > > Jacek
> > > >
> > > > czw., 2 kwi 2020 o 15:32 Antoine Pitrou <an...@python.org>
> > napisał(a):
> > > > >
> > > > >
> > > > > PS: in both cases, Cython files are not processed. autopep8 is
> > actually
> > > > > able to process them, but the comparison wouldn't be
> > apples-to-apples.
> > > > >
> > > > > (that said, autopep8 gives suboptimal results on Cython files, for
> > > > > example it changes "&c_variable" to "& c_variable" and
> > > > > "void* ptr" to "void * ptr")
> > > > >
> > > > > Regards
> > > > >
> > > > > Antoine.
> > > > >
> > > > > Le 02/04/2020 à 15:30, Antoine Pitrou a écrit :
> > > > > >
> > > > > > Hello,
> > > > > >
> > > > > > I've put up two PRs to compare the effect of running black vs.
> > autopep8
> > > > > > on the Python codebase.
> > > > > >
> > > > > > * black: https://github.com/apache/arrow/pull/6810
> > > > > > 65 files changed, 7855 insertions(+), 5215 deletions(-)
> > > > > >
> > > > > > * autopep8: https://github.com/apache/arrow/pull/6811
> > > > > > 20 files changed, 137 insertions(+), 118 deletions(-)
> > > > > >
> > > > > > I've configured black to try and minimize changes (for example,
> > avoid
> > > > > > normalizing string quoting style). Still, the number of changes is
> > > > > > humongous and they add 2600 lines to the codebase (which is a
> > tangible
> > > > > > amount of vertical space).
> > > > > >
> > > > > > Regards
> > > > > >
> > > > > > Antoine.
> > > > > >
> >
Re: [Python] black vs. autopep8
Posted by Neal Richardson <ne...@gmail.com>.
So autopep8 doesn't fix everything? Sounds inferior to me. That said, I'm
in favor of any resolution that increases our automation of this and
decreases the energy we expend debating it.
Neal
On Wed, Apr 8, 2020 at 10:34 AM Wes McKinney <we...@gmail.com> wrote:
> Circling back on this, it seems there isn't consensus about switching
> to Black, and using autopep8 at least will give us an easy way to
> maintain PEP8 compliance and help contributors fix linting failures
> detected by flake8 (but not all, e.g. unused imports would need to be
> manually removed). Would everyone be on board with using autopep8?
>
> On Thu, Apr 2, 2020 at 9:07 AM Wes McKinney <we...@gmail.com> wrote:
> >
> > I'm personally fine with the Black changes. After the one-time cost of
> > reformatting the codebase, it will take any personal preferences out
> > of code formatting (I admit that I have several myself, but I don't
> > mind the normalization provided by Black). I hope that Cython support
> > comes soon since a great deal of our code is Cython
> >
> > On Thu, Apr 2, 2020 at 9:00 AM Jacek Pliszka <ja...@gmail.com>
> wrote:
> > >
> > > Hi!
> > >
> > > I believe amount of changes is not that important.
> > >
> > > In my opinion, what matters is which format will allow reviewers to be
> > > more efficient.
> > >
> > > The committer can always reformat as they like. It is harder for the
> reviewer.
> > >
> > > BR,
> > >
> > > Jacek
> > >
> > > czw., 2 kwi 2020 o 15:32 Antoine Pitrou <an...@python.org>
> napisał(a):
> > > >
> > > >
> > > > PS: in both cases, Cython files are not processed. autopep8 is
> actually
> > > > able to process them, but the comparison wouldn't be
> apples-to-apples.
> > > >
> > > > (that said, autopep8 gives suboptimal results on Cython files, for
> > > > example it changes "&c_variable" to "& c_variable" and
> > > > "void* ptr" to "void * ptr")
> > > >
> > > > Regards
> > > >
> > > > Antoine.
> > > >
> > > > Le 02/04/2020 à 15:30, Antoine Pitrou a écrit :
> > > > >
> > > > > Hello,
> > > > >
> > > > > I've put up two PRs to compare the effect of running black vs.
> autopep8
> > > > > on the Python codebase.
> > > > >
> > > > > * black: https://github.com/apache/arrow/pull/6810
> > > > > 65 files changed, 7855 insertions(+), 5215 deletions(-)
> > > > >
> > > > > * autopep8: https://github.com/apache/arrow/pull/6811
> > > > > 20 files changed, 137 insertions(+), 118 deletions(-)
> > > > >
> > > > > I've configured black to try and minimize changes (for example,
> avoid
> > > > > normalizing string quoting style). Still, the number of changes is
> > > > > humongous and they add 2600 lines to the codebase (which is a
> tangible
> > > > > amount of vertical space).
> > > > >
> > > > > Regards
> > > > >
> > > > > Antoine.
> > > > >
>
Re: [Python] black vs. autopep8
Posted by Wes McKinney <we...@gmail.com>.
Circling back on this, it seems there isn't consensus about switching
to Black, and using autopep8 at least will give us an easy way to
maintain PEP8 compliance and help contributors fix linting failures
detected by flake8 (but not all, e.g. unused imports would need to be
manually removed). Would everyone be on board with using autopep8?
On Thu, Apr 2, 2020 at 9:07 AM Wes McKinney <we...@gmail.com> wrote:
>
> I'm personally fine with the Black changes. After the one-time cost of
> reformatting the codebase, it will take any personal preferences out
> of code formatting (I admit that I have several myself, but I don't
> mind the normalization provided by Black). I hope that Cython support
> comes soon since a great deal of our code is Cython
>
> On Thu, Apr 2, 2020 at 9:00 AM Jacek Pliszka <ja...@gmail.com> wrote:
> >
> > Hi!
> >
> > I believe amount of changes is not that important.
> >
> > In my opinion, what matters is which format will allow reviewers to be
> > more efficient.
> >
> > The committer can always reformat as they like. It is harder for the reviewer.
> >
> > BR,
> >
> > Jacek
> >
> > czw., 2 kwi 2020 o 15:32 Antoine Pitrou <an...@python.org> napisał(a):
> > >
> > >
> > > PS: in both cases, Cython files are not processed. autopep8 is actually
> > > able to process them, but the comparison wouldn't be apples-to-apples.
> > >
> > > (that said, autopep8 gives suboptimal results on Cython files, for
> > > example it changes "&c_variable" to "& c_variable" and
> > > "void* ptr" to "void * ptr")
> > >
> > > Regards
> > >
> > > Antoine.
> > >
> > > Le 02/04/2020 à 15:30, Antoine Pitrou a écrit :
> > > >
> > > > Hello,
> > > >
> > > > I've put up two PRs to compare the effect of running black vs. autopep8
> > > > on the Python codebase.
> > > >
> > > > * black: https://github.com/apache/arrow/pull/6810
> > > > 65 files changed, 7855 insertions(+), 5215 deletions(-)
> > > >
> > > > * autopep8: https://github.com/apache/arrow/pull/6811
> > > > 20 files changed, 137 insertions(+), 118 deletions(-)
> > > >
> > > > I've configured black to try and minimize changes (for example, avoid
> > > > normalizing string quoting style). Still, the number of changes is
> > > > humongous and they add 2600 lines to the codebase (which is a tangible
> > > > amount of vertical space).
> > > >
> > > > Regards
> > > >
> > > > Antoine.
> > > >
Re: [Python] black vs. autopep8
Posted by Wes McKinney <we...@gmail.com>.
I'm personally fine with the Black changes. After the one-time cost of
reformatting the codebase, it will take any personal preferences out
of code formatting (I admit that I have several myself, but I don't
mind the normalization provided by Black). I hope that Cython support
comes soon since a great deal of our code is Cython
On Thu, Apr 2, 2020 at 9:00 AM Jacek Pliszka <ja...@gmail.com> wrote:
>
> Hi!
>
> I believe amount of changes is not that important.
>
> In my opinion, what matters is which format will allow reviewers to be
> more efficient.
>
> The committer can always reformat as they like. It is harder for the reviewer.
>
> BR,
>
> Jacek
>
> czw., 2 kwi 2020 o 15:32 Antoine Pitrou <an...@python.org> napisał(a):
> >
> >
> > PS: in both cases, Cython files are not processed. autopep8 is actually
> > able to process them, but the comparison wouldn't be apples-to-apples.
> >
> > (that said, autopep8 gives suboptimal results on Cython files, for
> > example it changes "&c_variable" to "& c_variable" and
> > "void* ptr" to "void * ptr")
> >
> > Regards
> >
> > Antoine.
> >
> > Le 02/04/2020 à 15:30, Antoine Pitrou a écrit :
> > >
> > > Hello,
> > >
> > > I've put up two PRs to compare the effect of running black vs. autopep8
> > > on the Python codebase.
> > >
> > > * black: https://github.com/apache/arrow/pull/6810
> > > 65 files changed, 7855 insertions(+), 5215 deletions(-)
> > >
> > > * autopep8: https://github.com/apache/arrow/pull/6811
> > > 20 files changed, 137 insertions(+), 118 deletions(-)
> > >
> > > I've configured black to try and minimize changes (for example, avoid
> > > normalizing string quoting style). Still, the number of changes is
> > > humongous and they add 2600 lines to the codebase (which is a tangible
> > > amount of vertical space).
> > >
> > > Regards
> > >
> > > Antoine.
> > >
Re: [Python] black vs. autopep8
Posted by Jacek Pliszka <ja...@gmail.com>.
Hi!
I believe amount of changes is not that important.
In my opinion, what matters is which format will allow reviewers to be
more efficient.
The committer can always reformat as they like. It is harder for the reviewer.
BR,
Jacek
czw., 2 kwi 2020 o 15:32 Antoine Pitrou <an...@python.org> napisał(a):
>
>
> PS: in both cases, Cython files are not processed. autopep8 is actually
> able to process them, but the comparison wouldn't be apples-to-apples.
>
> (that said, autopep8 gives suboptimal results on Cython files, for
> example it changes "&c_variable" to "& c_variable" and
> "void* ptr" to "void * ptr")
>
> Regards
>
> Antoine.
>
> Le 02/04/2020 à 15:30, Antoine Pitrou a écrit :
> >
> > Hello,
> >
> > I've put up two PRs to compare the effect of running black vs. autopep8
> > on the Python codebase.
> >
> > * black: https://github.com/apache/arrow/pull/6810
> > 65 files changed, 7855 insertions(+), 5215 deletions(-)
> >
> > * autopep8: https://github.com/apache/arrow/pull/6811
> > 20 files changed, 137 insertions(+), 118 deletions(-)
> >
> > I've configured black to try and minimize changes (for example, avoid
> > normalizing string quoting style). Still, the number of changes is
> > humongous and they add 2600 lines to the codebase (which is a tangible
> > amount of vertical space).
> >
> > Regards
> >
> > Antoine.
> >
Re: [Python] black vs. autopep8
Posted by Antoine Pitrou <an...@python.org>.
PS: in both cases, Cython files are not processed. autopep8 is actually
able to process them, but the comparison wouldn't be apples-to-apples.
(that said, autopep8 gives suboptimal results on Cython files, for
example it changes "&c_variable" to "& c_variable" and
"void* ptr" to "void * ptr")
Regards
Antoine.
Le 02/04/2020 à 15:30, Antoine Pitrou a écrit :
>
> Hello,
>
> I've put up two PRs to compare the effect of running black vs. autopep8
> on the Python codebase.
>
> * black: https://github.com/apache/arrow/pull/6810
> 65 files changed, 7855 insertions(+), 5215 deletions(-)
>
> * autopep8: https://github.com/apache/arrow/pull/6811
> 20 files changed, 137 insertions(+), 118 deletions(-)
>
> I've configured black to try and minimize changes (for example, avoid
> normalizing string quoting style). Still, the number of changes is
> humongous and they add 2600 lines to the codebase (which is a tangible
> amount of vertical space).
>
> Regards
>
> Antoine.
>