fix(lint): change max line length to 96 for linters

[jonburdo]

This makes the following changes:

• changes the max line length to 96 (from 79)
• applies changes made by running black after change the max line length and a few of small manual fixes
• removes the add-trailing-comma pre-commit hook, which was also increasing vertical spread

The max line length was originally 79, causing a bit more vertical spread than we wanted and being kind of a nuisance at times. Many projects have a higher limit for good reason (see black's line length section). Somewhere in the 85-100 seems like a better balance.

Removing add-trailing-comma mainly just allows black to use the following format:

func(
    1, 2, 3
)

in cases where the arguments can fit on one line but the entire expression (i.e. func(1, 2, 3)) cannot.

fixes #499

[skshetry]

Thank you so much for making this change. 🙏🏼 Is there a reason why you picked 96 instead of letting black decide?

[jonburdo]

Thank you so much for making this change. 🙏🏼 Is there a reason why you picked 96 instead of letting black decide?

Good question. I took the approach of just running black with a few different numbers and looking at the diff. I went ahead and picked the largest number that still seemed reasonable and readable. I figured we can always adjust again later if needed. I actually kind of like the idea the black docs suggest of using Bugbear to allow lines that black can't autoformat to go a little above the max. I just figured I'd keep the config changes minimal for the moment.

A couple more specific reasons that I like pushing a bit over 88 to something like 96:

• We have some long variable and function names in an effort to be descriptive. A short max results in things like this (from ldb/index/base.py):

      (
          local_storage_files,
          ephemeral_files,
      ) = separate_storage_and_non_storage_files(
          possibly_ephemeral_files,
          storage_locations,
      )

  Maybe code structure and being overly descriptive is the real problem here, but I figure we can see how it goes for now with a longer max line length.

• Having functions with say 12 or more expressions is very reasonable and pretty common in which case there's a real cost to unnecessary vertical spread.

I would definitely be interested to hear other opinions on this. Is 96 maybe getting too long? What feels like the best balance?

[skshetry]

I actually kind of like the idea the black docs suggest of using Bugbear to allow lines that black can't autoformat to go a little above the max. I just figured I'd keep the config changes minimal for the moment.

I like the idea too, but I am not sure how that works in practice.

A couple more specific reasons that I like pushing a bit over 88 to something like 96:

• We have some long variable and function names in an effort to be descriptive. A short max results in things like this (from ldb/index/base.py):

    (
        local_storage_files,
        ephemeral_files,
    ) = separate_storage_and_non_storage_files(
        possibly_ephemeral_files,
        storage_locations,
    )

In DVC, we try to be succinct as possible, so you'll find a lot of APIs that are usually one word or two, and we leave a lot to the surrounding context/namespace.

Here, since the context is process_files(), we can leave out _files from the variables, and probably storage_locations to just storages or locations.

local, ephemeral = separate_storage_and_non_storage_files(files, storages)
# or,
local, ephemeral = split_storage_and_non_storage_files(files, storages)

Also, see Studio BE review cheatsheet regarding this.

Is 96 maybe getting too long? What feels like the best balance?

FYI, Studio uses 100 as a line length. For me, 88 is 10% more than 79 already. 96 works for me too, no strong opinion. It's just that I find it easier to argue in the future if we leave it to the formatter's default.

[jonburdo]

Thanks @skshetry ! I agree with all those points

[skshetry]

Just to clarify, I'm okay with 96 or any line length that you decide. I was only curious about the reason. 🙂

[jonburdo]

Just to clarify, I'm okay with 96 or any line length that you decide. I was only curious about the reason. slightly_smiling_face

Yeah, that's what I got from what you said - makes sense :) We'll merge this as is for now, but may adjust later