aiida-core icon indicating copy to clipboard operation
aiida-core copied to clipboard

120 too long. Let's discuss.

Open edan-bainglass opened this issue 2 months ago • 5 comments

I'm aware 120 is a common line-length standard in the Python community. However, I believe it negatively impacts readability. I propose we shift it down to 88, ~~or dare I say 80~~.

But good to get the pro-120 argument also. Anyone?

edan-bainglass avatar Sep 29 '25 07:09 edan-bainglass

I was thinking the same recently. My main qualm is that on most screens, I'm having a hard time fitting two open files next to each other and not having to scroll to see the whole line. Not everyone uses the tiny font size @sphuber can with his elvish Dutch eyes.

I think historically formatters did some pretty funky things when you reduced the line length. But I just tried changing to 88 characters and for most code snippets that changed I agree the clarity has been improved, e.g.:

    for key, value in source.items():
        if key in original and isinstance(value, (Dict, Mapping)) and isinstance(original[key], (Dict, Mapping)):
            original[key] = update_mapping(original[key], value)

Kind of dense, a bit hard to parse. And have to scroll on GitHub, bah! This becomes:

    for key, value in source.items():
        if (
            key in original
            and isinstance(value, (Dict, Mapping))
            and isinstance(original[key], (Dict, Mapping))
        ):
            original[key] = update_mapping(original[key], value)

Which I find more readable. So +1 from me, and I will probably put this in the pipeline for aiida-quantumespresso.

Some other notes:

  • I thought we would have a lot of work updating (doc)strings, but apparently Ruff doesn't make a fuss about these?
  • Seems the formatter does mess up comments to skip rules (# noqa), so there might be a bit of work there.

mbercx avatar Sep 29 '25 23:09 mbercx

Thanks @mbercx. Your description accurately captures my thoughts. I also do not use my TV as my work screen.

Not everyone uses the tiny font size @sphuber can with his elvish Dutch eyes.

I recall once @sphuber sharing his screen over a call and my brain felt like it auto-zoomed out at lightspeed 😵‍💫

edan-bainglass avatar Oct 03 '25 04:10 edan-bainglass

Let's discuss this during the core days :3 This is just a reminder that, if we go through with this change (which I could get on board with), that we add the commit to the .git-blame-ignore-revs file, either by doing a merge-commit, not squash-merge, or via a separate PR that updates .git-blame-ignore-revs.

GeigerJ2 avatar Oct 03 '25 08:10 GeigerJ2

Some other notes:

  • I thought we would have a lot of work updating (doc)strings, but apparently Ruff doesn't make a fuss about these?
  • Seems the formatter does mess up comments to skip rules (# noqa), so there might be a bit of work there.

https://pypi.org/project/docformatter/ at least for docstring

edan-bainglass avatar Oct 04 '25 09:10 edan-bainglass

https://pypi.org/project/docformatter/ at least for docstring

My local testing of docformatter are hit and miss, though the miss follows a pattern. I'll open an issue on docformatter, but for now, I wouldn't rely on it.

edan-bainglass avatar Oct 05 '25 10:10 edan-bainglass