Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Need to establish standards for code linting #58

Open
emcangi opened this issue Feb 9, 2024 · 3 comments
Open

Need to establish standards for code linting #58

emcangi opened this issue Feb 9, 2024 · 3 comments
Assignees

Comments

@emcangi
Copy link
Collaborator

emcangi commented Feb 9, 2024

Code linting varies between individual computers/programming environments, and norms can also vary between Python packages. We need to agree upon a code linting standard to avoid introducing disagreements on spacing, carriage returns, etc. that can lead to confusing commits if linting changes are accidentally made along with other content changes.

Noted by #54 (comment)

@planetarymike
Copy link
Collaborator

What linter do you use? I'm set up to use flake8, but could switch to pylint or ruff easily, and to other with more difficulty.

@emcangi
Copy link
Collaborator Author

emcangi commented Feb 13, 2024

I'm on flake8 also in Sublime Text using Sublime Linter, so it should be giving us the same feedback! I've never even heard of ruff.

I think the main issues were styling choices that aren't covered by linting, and are instead left up to the individual module maintainers to establish conventions, especially where to begin docstrings, either on the same line as """ or the following line. I just personally dislike having the text start on the same line so I would compulsively change it without realizing, but this style I think is a numpy standard that many packages adopt. I could be forced to accept this relatively benign style though.

There are some other inconsistent styling things that we ignored for now that should be caught by the linter, and could be captured in one large "styling update" commit/PR:

  • Whitespace around operators is inconsistent
  • Double carriage return between functions or between imports and functions (mostly present, but some anomalous files)
  • Line length, which relates to whether to print each argument to a multi-argument function call or a definition on a new line: I realize now in writing this that I set my Sublime Text to ignore the line length rule because it was giving me anxiety, as it was something I didn't have time to mess with while trying to defend. I could turn it back on though.

@planetarymike
Copy link
Collaborator

If we're both using flake8, great! At that point the main thing is to not make formatting changes unless you're also making substantive changes to the content of the lines.

We do need to decide on a line length limit and enforce it; 120 characters?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants