pyElli icon indicating copy to clipboard operation
pyElli copied to clipboard

Consistent code style

Open MarJMue opened this issue 3 years ago • 9 comments

Run all subfiles through a linter and an formatter, to get a consistent style.

Part of the code is already formatted with black, so it should be used everywhere.

MarJMue avatar Jun 29 '22 21:06 MarJMue

Should we maybe also build an Action to check linting?

domna avatar Jun 30 '22 06:06 domna

👍

MarJMue avatar Jun 30 '22 06:06 MarJMue

Formatted according to black and added actions for black linting and sphinx-lint

domna avatar Jul 26 '22 19:07 domna

@MarJMue Shall we enforce pylint as pre-commit hook and GitHub action? Sometimes I find pylint a little over-restrictive but we can deactivate it with # pylint: disable=... to show that we deliberately did something else.

domna avatar Jul 26 '22 19:07 domna

I am not really sure. I think pylint has a lot going for it, but as you said there are a more then a few false positives and I do not like to add even more comment based command in the source code, e.g. before every single character variable taken from literature.

Maybe if we disable a few selected warnings...

MarJMue avatar Jul 26 '22 19:07 MarJMue

Yes, I think we should disable some warnings globally and add the short names to the --good-names options when running pylint to accept them. Then it should mainly work w/o any inline comments to the code.

domna avatar Jul 26 '22 20:07 domna

I added a pylintrc and we now support the regex pattern [a-zA-Z][a-zA-Z0-9_]{0,30}$ for names of variables, arguments and attributes throughout the whole project. Can be used with --rcfile=.pylintrc when invoking pylint, but vscode picks it up by default.

domna avatar Jul 27 '22 09:07 domna

Looks really good. But i will still try to remove any unnecessary uses of single character variables, while writing more documentation.

MarJMue avatar Jul 28 '22 09:07 MarJMue

Yes, I think it's generally good to try to still follow the general naming style. But there really are plenty of variable names which are just naturally written in another style in physics which I would not want to break (e.g. Eg).

I still have to refine the regex pattern as well, e.g. we should allow underscores in the beginning for attributes to mark them as "private". But I at some point I will sit down and go through the pylinting issues and try to fix them and then we can introduce an action for checking it.

domna avatar Jul 28 '22 09:07 domna