Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-649

Allow formatting python code with "black"

    XMLWordPrintable

    Details

    • Type: RFC
    • Status: Withdrawn
    • Resolution: Done
    • Component/s: DM
    • Labels:
      None
    • Location:
      This ticket

      Description

      A good automatic code formatter has the potential to save a lot of developer time vs. hand formatting, testing with a validator, and learning to read different user's styles.

      The "black" formatter https://black.readthedocs.io/en/stable/ seems to do a very nice job with Python code (unlike AutoPep8) and the results appear to be pep8 complaint. The one major difference I have noticed compared to our standards is that when a statement with binary operators is broken into multiple lines, the binary operator is at the beginning of the new line. flake8 can be configured to support this.

      Note that black is not very configurable. So we don't have to argue about which settings to use.

      This proposal is to allow us to use black to format our code. A future RFC would be needed to require use of black.

      Thanks to Matthew Becker for bring it to my attention.

      As an experiment I used black (with no configuration) to reformat all of the python code in ts_salobj (a fairly large package of pure Python code). I then ran flake8 on it only got two categories of warnings:

      • W503 line break before binary operator. This is expected and reflects an existing standard we would have to change.
      • E203 whitespace before ':'. Black uses [start : end] instead of [start: end] for ranges.

      I made a pull request to show what changed: https://github.com/lsst-ts/ts_salobj/pull/69
      I don't agree with all the changes, but I think the results are quite readable. I think the potential productivity benefits far outweigh any minor formatting annoyances.

        Attachments

          Issue Links

            Activity

            Hide
            rowen Russell Owen added a comment -

            I filed https://jira.lsstcorp.org/browse/RFC-650 to change our standard for binary operators in continuation lines. But as I said, I see it as orthogonal, though it would nicely pave the way for this one.

            Show
            rowen Russell Owen added a comment - I filed https://jira.lsstcorp.org/browse/RFC-650 to change our standard for binary operators in continuation lines. But as I said, I see it as orthogonal, though it would nicely pave the way for this one.
            Hide
            rowen Russell Owen added a comment - - edited

            John Parejko my experience has apparently been quite different than yours. I find that when I rename variables or functions (e.g. as in response to your review of ts_simactuators) I often have to manually reindent a lot of code – especially a nuisance when arguments appear on new lines, which are rarely at a multiple of 4 characters. Another place where I have wasted far too much time reformatting is converting asynchronous unit tests to use asynctest; this requires deleting two lines and outdenting a block of code for every test. The deletion is a simple search and replace, but the outdenting is a waste of time. I spend all day writing Python code and I feel I waste far too much time formatting it.

            Show
            rowen Russell Owen added a comment - - edited John Parejko my experience has apparently been quite different than yours. I find that when I rename variables or functions (e.g. as in response to your review of ts_simactuators) I often have to manually reindent a lot of code – especially a nuisance when arguments appear on new lines, which are rarely at a multiple of 4 characters. Another place where I have wasted far too much time reformatting is converting asynchronous unit tests to use asynctest ; this requires deleting two lines and outdenting a block of code for every test. The deletion is a simple search and replace, but the outdenting is a waste of time. I spend all day writing Python code and I feel I waste far too much time formatting it.
            Hide
            swinbank John Swinbank added a comment -

            Flagged by the DM-CCB.

            Show
            swinbank John Swinbank added a comment - Flagged by the DM-CCB.
            Hide
            swinbank John Swinbank added a comment -

            We discussed this at the DM-CCB of 2019-12-11. We do not approve this RFC as written.

            If the Telescope & Site team were to adopt Black, develop appropriate procedures and infrastructures around it, and demonstrate that it had been a big win for productivity, the DM-CCB would be prepared to reconsider.

            Show
            swinbank John Swinbank added a comment - We discussed this at the DM-CCB of 2019-12-11. We do not approve this RFC as written. If the Telescope & Site team were to adopt Black, develop appropriate procedures and infrastructures around it, and demonstrate that it had been a big win for productivity, the DM-CCB would be prepared to reconsider.
            Hide
            tjenness Tim Jenness added a comment -

            Russell Owen DM-CCB are withdrawing this RFC. If you desire, you can reopen this RFC as a T&S RFC.

            Show
            tjenness Tim Jenness added a comment - Russell Owen DM-CCB are withdrawing this RFC. If you desire, you can reopen this RFC as a T&S RFC.

              People

              Assignee:
              rowen Russell Owen
              Reporter:
              rowen Russell Owen
              Watchers:
              Brian Van Klaveren, Jim Bosch, John Parejko, John Swinbank, Jonathan Sick, Krzysztof Findeisen, Leanne Guy, Russell Owen, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Planned End:

                  Jenkins

                  No builds found.