-
Notifications
You must be signed in to change notification settings - Fork 170
Description
I think we should discuss whether to enforce some of PEP8 variable naming conventions in our codebase.
This can be done via the 'pep8-naming (N)' ruleset from Ruff.
Looking at our current codebase, enabling the N rule with have the following violations
{'N802': 193,
'N803': 21,
'N805': 4,
'N806': 478,
'N811': 2,
'N816': 42,
'N818': 2}
Repro instructions
diff --git a/pyproject.toml b/pyproject.toml
index 86e1420b..4ce41eb6 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -85,6 +85,7 @@ select = [
"ISC001", # single-line-implicit-string-concatenation
"TID", # flake8-tidy-imports
"T100", # Checks for the presence of debugger calls and imports
+ "N", # pep8 naming conventions
]
ignore = [
then doing
ruff check . | grep -E '^N\d{3}' > pep-log.txt && python process-log.py
where
# process-log.py
from pathlib import Path
FILE = 'pep-log.txt'
with open(FILE,'r') as f:
errors = f.read().splitlines()
codes = {}
for error in errors:
code = error.split(" ")[0]
if code not in codes:
codes[code] = 0
codes[code] += 1
from pprint import pprint
pprint(codes)As with all Ruff rules, we should be selective about which ones to follow. Some conventions simply do not apply in certain domains (e.g., in numpy, X = np.array(...) is commonly used for multidimensional arrays, and so is dX - even though it violates PEP8) .
In Parcels, we have kernels following CamelCase naming. This is also a pep8 violation as functions aren't named so, but is really useful since its clear what's a kernel and I don't think we should change this convention.
I'll put together a PR that selects which of these rules to enforce and which to ignore - striking a balance that works best for Parcels.
Metadata
Metadata
Assignees
Labels
Type
Projects
Status