I will attempt to explain how I differentiate these things in the vain hope that this may make it easier for us to discuss these things. I say "vain" because I will probably fail due to my own communication abilities. I think these things are hard to distinguish due to their historical development in programming culture.
First, if you'll indulge me, when it comes to processing code-as-text, I think of the following 3 things with these definitions:
- Linting: True "error-checking", as in using an external program to check that your code file will not run if you try to execute it.
- Formatting: How the text looks in a text file. This is independent of whether it runs or not.
- "Programming style guide": How the code is organized, on a meta-level.
I will give explicit examples of each. Let's say we've got a file named example_lint.py located in hnn-core/hnn_core with the following contents:
import warnings
from externals.mne import _check_option
def simulate_dipole(net, tstop, dt=0.025, n_trials=None, record_vsec=False,
record_isec=False, record_ca=False, postproc=False):
"""No docstring for brevity.
"""
from .parallel_backends import _BACKEND, JoblibBackend
if _BACKEND is None:
_BACKEND = JoblibBackend(n_jobs=1)
if n_trials is None:
n_trials = net._params['N_trials']
if n_trials < 1:
raise ValueError("Invalid number of simulations: %d" % n_trials)
if not net.connectivity:
warnings.warn('No connections instantiated in network. Consider using '
'net = jones_2009_model() or net = law_2021_model() to '
'create a predefined network from published models.',
UserWarning)
# ADD DRIVE WARNINGS HERE
if not net.external_drives and not net.external_biases:
warnings.warn('No external drives or biases loaded', UserWarning)
for drive_name, drive in net.external_drives.items():
if 'tstop' in drive['dynamics']:
if drive['dynamics']['tstop'] is None:
drive['dynamics']['tstop'] = tstop
for bias_name, bias in net.external_biases.items():
for cell_type, bias_cell_type in bias.items():
if bias_cell_type['tstop'] is None:
bias_cell_type['tstop'] = tstop
if bias_cell_type['tstop'] < 0.:
raise ValueError('End time of tonic input cannot be negative')
duration = bias_cell_type['tstop'] - bias_cell_type['t0']
if duration < 0.:
raise ValueError('Duration of tonic input cannot be negative')
net._instantiate_drives(n_trials=n_trials, tstop=tstop)
net._reset_rec_arrays()
_check_option('record_vsec', record_vsec, ['all', 'soma', False])
net._params['record_vsec'] = record_vsec
_check_option('record_isec', record_isec, ['all', 'soma', False])
net._params['record_isec'] = record_isec
_check_option('record_ca', record_ca, ['all', 'soma', False])
net._params['record_ca'] = record_ca
net._tstop = tstop
net._dt = dt
if postproc:
warnings.warn('The postproc-argument is deprecated and will be removed'
' in a future release of hnn-core. Please define '
'smoothing and scaling explicitly using Dipole methods.',
DeprecationWarning)
dpls = _BACKEND.simulate(net, tstop, dt, n_trials, postproc)
return dpls
if __name__ == "__main__":
print('Hello world!')
If we run linters on this file, for example running either flake8 example_lint.py or ruff check example_lint.py, the file will not be modified, and we will see output complaining about SyntaxErrors or E999, such as in the following output:
(hc12) [17:59] ~hc [remotes/samadpls/feat/batch_simulation] > flake8 example_lint.py
example_lint.py:6:2: E999 IndentationError: unexpected indent
(hc12) [17:59] ~hc [remotes/samadpls/feat/batch_simulation] > ruff check example_lint.py
example_lint.py:6:1: SyntaxError: Unexpected indentation
|
6 | def simulate_dipole(net, tstop, dt=0.025, n_trials=None, record_vsec=False,
| ^
7 | record_isec=False, record_ca=False, postproc=False):
8 | """No docstring for brevity.
|
example_lint.py:71:1: SyntaxError: Expected a statement
|
69 | return dpls
70 |
71 | if __name__ == "__main__":
| ^
72 | print('Hello world!')
|
Found 2 errors.
Our linters have identified an actual error that would prevent this code from running at all: we had an extra space in front of def. We can look up exactly what this "error ID" (my term) is using both the flake8 documentation (here https://flake8.pycqa.org/en/latest/user/error-codes.html ) or the ruff documentation (here https://docs.astral.sh/ruff/rules/#pyflakes-f ). In some cases of errors like this, some linters can automatically "fix" the error by passing in a special argument (not flake8 however). However, this doesn't work in all cases, and both Mainak and I agree that it is bad practice to rely on a tool to fix program-breaking errors. Instead, you should fix it, and understand the source of the bug, especially in case the origin is important. Note that most IDEs will constantly be running linting programs in the background every time you make a code change, in order to detect exactly these kind of things.
Indeed, we see that the file will not execute correctly:
(hc12) [17:48] ~hc [remotes/samadpls/feat/batch_simulation] > python example_lint.py
File "/home/<REDACTED>/example_lint.py", line 6
def simulate_dipole(net, tstop, dt=0.025, n_trials=None, record_vsec=False,
IndentationError: unexpected indent
Let's pretend now that I go in and quickly (too quickly) fix the bug that our linters found. I will rename the file to example_format.py, and now it looks like this:
import warnings
from externals.mne import _check_option
def simulate_dipole(net, tstop, dt=0.025, n_trials=None, record_vsec=False,
record_isec=False, record_ca=False, postproc=False):
"""No docstring for brevity.
"""
from .parallel_backends import _BACKEND, JoblibBackend
if _BACKEND is None:
_BACKEND = JoblibBackend(n_jobs=1)
if n_trials is None:
n_trials = net._params['N_trials']
if n_trials < 1:
raise ValueError("Invalid number of simulations: %d" % n_trials)
if not net.connectivity:
warnings.warn('No connections instantiated in network. Consider using '
'net = jones_2009_model() or net = law_2021_model() to '
'create a predefined network from published models.',
UserWarning)
# ADD DRIVE WARNINGS HERE
if not net.external_drives and not net.external_biases:
warnings.warn('No external drives or biases loaded', UserWarning)
for drive_name, drive in net.external_drives.items():
if 'tstop' in drive['dynamics']:
if drive['dynamics']['tstop'] is None:
drive['dynamics']['tstop'] = tstop
for bias_name, bias in net.external_biases.items():
for cell_type, bias_cell_type in bias.items():
if bias_cell_type['tstop'] is None:
bias_cell_type['tstop'] = tstop
if bias_cell_type['tstop'] < 0.:
raise ValueError('End time of tonic input cannot be negative')
duration = bias_cell_type['tstop'] - bias_cell_type['t0']
if duration < 0.:
raise ValueError('Duration of tonic input cannot be negative')
net._instantiate_drives(n_trials=n_trials, tstop=tstop)
net._reset_rec_arrays()
_check_option('record_vsec', record_vsec, ['all', 'soma', False])
net._params['record_vsec'] = record_vsec
_check_option('record_isec', record_isec, ['all', 'soma', False])
net._params['record_isec'] = record_isec
_check_option('record_ca', record_ca, ['all', 'soma', False])
net._params['record_ca'] = record_ca
net._tstop = tstop
net._dt = dt
if postproc:
warnings.warn('The postproc-argument is deprecated and will be removed'
' in a future release of hnn-core. Please define '
'smoothing and scaling explicitly using Dipole methods.',
DeprecationWarning)
dpls = _BACKEND.simulate(net, tstop, dt, n_trials, postproc)
return dpls
if __name__ == "__main__":
print('Hello world!')
If I execute the file, it works!
(hc12) [18:00] ~hc [remotes/samadpls/feat/batch_simulation] > python example_format.py
Hello world!
However, if I run it through the flake8 linter, I run into problems:
(hc12) [18:02] ~hc [remotes/samadpls/feat/batch_simulation] > flake8 example_format.py
example_format.py:5:1: E302 expected 2 blank lines, found 1
example_format.py:70:1: E305 expected 2 blank lines after class or function definition, found 1
The reason I'm being so pedantic thus far is that Linting vs Formatting, in actual practice, is "muddled", and the separation between them is not commonly accepted in the public lexicon. Linting originally https://en.wikipedia.org/wiki/Lint_%28software%29 was only applied to error-checking. However, as the wikipedia article points out, "Lint-like tools have also been developed for other aspects of software development: enforcing grammar and style guides for given language source code." Both flake8 and ruff do this: their linters (the flake8 and ruff check executables) not only detect actual bugs like the SyntaxError we had before, but also style violations.
In the case of flake8's error codes (see https://flake8.pycqa.org/en/latest/user/error-codes.html ), it mentions at the bottom that it also relies on the error codes provided by pycodestyle, here: https://pycodestyle.readthedocs.io/en/latest/intro.html#error-codes . Our E302 and E305 error codes are listed in the pycodestyle site, and that is what flake8 has detected.
It should be noted that the term "error code" is a little ambiguous here. In the case of the F-series of error codes here https://flake8.pycqa.org/en/latest/user/error-codes.html , many of these are "error codes" describing a type of error that will prevent your program from running. However, OTHER error codes that flake8 and ruff provide are NOT necessarily "codes that identify the error your program will run into". Instead, they are "error codes" in the pydocstyle sense ( https://pycodestyle.pycqa.org/en/latest/intro.html#error-codes ), which is that that they are uniquely-identified PEP8 style violations, such as the E-series shown in that link. In these cases, your code will run fine, but it violates the most widely-held (and most permissible) standard in Python code formatting, PEP8.
So, to be clear, flake8, as with most "linters", handles both actual error-checking (in the program bug sense), AND formatting style violation/"error" checking. In general, in the case of linters, it is difficult to infer "is this error a real code error or just a style error?" without already knowing how to read the class of error, etc.
Like I said before, Mainak and I agree that if your linter can auto-fix errors, you should generally NOT rely on that to fix "true" errors ("bug-errors", as in the program bug sense).
However, auto-fixing of Formatting style violations (or "style-errors") are a different story. In principle, you should be able to trust that a "style-reformatter" program will NOT introduce new bugs, if you tell it to simply re-Format the code according to a formatting guide like PEP8. flake8 cannot auto-fix any errors (bug or style). For style-errors, traditionally, you might use something like autopep8. For example, if we copied our file example_format.py to a new one named example_format_autopep8.py, then ran autopep8 --in-place example_format_autopep8.py, we would get a new file where the only differences are shown here (output from running autopep8 --diff example_format.py):
--- original/example_format.py
+++ fixed/example_format.py
@@ -1,6 +1,7 @@
import warnings
from externals.mne import _check_option
+
def simulate_dipole(net, tstop, dt=0.025, n_trials=None, record_vsec=False,
record_isec=False, record_ca=False, postproc=False):
@@ -67,5 +68,6 @@
return dpls
+
if __name__ == "__main__":
print('Hello world!')
This should finally give us a file that has no bug-errors and no PEP8-style-errors according to our linters and autopep8, which is what we find:
(hc12) [18:25] ~hc [remotes/samadpls/feat/batch_simulation] > flake8 example_format_autopep8.py
(hc12) [18:45] ~hc [remotes/samadpls/feat/batch_simulation] > autopep8 --diff example_format_autopep8.py
(hc12) [18:45] ~hc [remotes/samadpls/feat/batch_simulation] > ruff check example_format_autopep8.py
All checks passed!
I know it's taken a while to get here, but these are the main reasons I have raised this whole issue:
- We should introduce auto-"style-error"-fixers as part of our development workflow (but not auto-"bug-error"-fixers!)
- We should follow the rest of the ecosystem and choose a Formatting style close to the Black/
ruffstyle.
PEP8 as a style guide has a lot of rules, yes, but it still allows much freedom in how you format your code, including allowing it to be difficult to read on the rare occasion. Partly as a result of this, the Black format ( https://black.readthedocs.io/en/stable/the_black_code_style/index.html ), was born, alongside a program to apply its formatting. It is PEP8 compliant (except for a line length of 88), and can be thought of as a more opinionated subset of PEP8-compliant code format style guides. In addition to providing a linter using ruff check, ruff also provides a separate auto-formatter program via ruff format, which is "designed as a drop-in replacement for Black" ( https://docs.astral.sh/ruff/formatter/ ). We can think of ruff format as analagous to autopep8, but for the more-specific Black format.
The Black format has been around for a while and has become extremely popular, especially with the speed afforded by ruff (and its predecessors). Specifically, in our ecosystem, many use either Black or ruff (here https://github.com/astral-sh/ruff?tab=readme-ov-file#whos-using-ruff and here https://github.com/psf/black#used-by ), including MNE itself https://github.com/mne-tools/mne-python/commit/e81ec528a42ac687f3d961ed5cf8e25f236925b0 .
Because ruff has both a linter (ruff check) and a style-reformatter (ruff format), it also does some (but not complete) separation between "bug-errors" versus "style-errors". For example, if we run ruff check on our previous file example_format.py, we get no errors, unlike the style-errors that flake8 shows:
(hc12) [19:22] ~hc [remotes/samadpls/feat/batch_simulation] > ruff check example_format.py
All checks passed!
(hc12) [19:22] ~hc [remotes/samadpls/feat/batch_simulation] > flake8 example_format.py
example_format.py:5:1: E302 expected 2 blank lines, found 1
example_format.py:70:1: E305 expected 2 blank lines after class or function definition, found 1
This is because ruff mostly expects style errors to be handled using ruff format: "By default, Ruff [linter] enables Flake8's F rules, along with a subset of the E rules, omitting any stylistic rules that overlap with the use of a formatter, like ruff format or Black." ( https://docs.astral.sh/ruff/rules/ ). Ruff's linter, ruff check, does not by default modify code. However, because fixing "style-errors" should be "non-destructive" in the sense of not introducing new bugs, Ruff's formatter, ruff format does modify code in-place, similar to autopep8 --in-place. If we copy our previous file example_format.py to a new file named example_format_ruff.py and run ruff format example_format_ruff.py, we get the following:
import warnings
from externals.mne import _check_option
def simulate_dipole(
net,
tstop,
dt=0.025,
n_trials=None,
record_vsec=False,
record_isec=False,
record_ca=False,
postproc=False,
):
"""No docstring for brevity."""
from .parallel_backends import _BACKEND, JoblibBackend
if _BACKEND is None:
_BACKEND = JoblibBackend(n_jobs=1)
if n_trials is None:
n_trials = net._params["N_trials"]
if n_trials < 1:
raise ValueError("Invalid number of simulations: %d" % n_trials)
if not net.connectivity:
warnings.warn(
"No connections instantiated in network. Consider using "
"net = jones_2009_model() or net = law_2021_model() to "
"create a predefined network from published models.",
UserWarning,
)
# ADD DRIVE WARNINGS HERE
if not net.external_drives and not net.external_biases:
warnings.warn("No external drives or biases loaded", UserWarning)
for drive_name, drive in net.external_drives.items():
if "tstop" in drive["dynamics"]:
if drive["dynamics"]["tstop"] is None:
drive["dynamics"]["tstop"] = tstop
for bias_name, bias in net.external_biases.items():
for cell_type, bias_cell_type in bias.items():
if bias_cell_type["tstop"] is None:
bias_cell_type["tstop"] = tstop
if bias_cell_type["tstop"] < 0.0:
raise ValueError("End time of tonic input cannot be negative")
duration = bias_cell_type["tstop"] - bias_cell_type["t0"]
if duration < 0.0:
raise ValueError("Duration of tonic input cannot be negative")
net._instantiate_drives(n_trials=n_trials, tstop=tstop)
net._reset_rec_arrays()
_check_option("record_vsec", record_vsec, ["all", "soma", False])
net._params["record_vsec"] = record_vsec
_check_option("record_isec", record_isec, ["all", "soma", False])
net._params["record_isec"] = record_isec
_check_option("record_ca", record_ca, ["all", "soma", False])
net._params["record_ca"] = record_ca
net._tstop = tstop
net._dt = dt
if postproc:
warnings.warn(
"The postproc-argument is deprecated and will be removed"
" in a future release of hnn-core. Please define "
"smoothing and scaling explicitly using Dipole methods.",
DeprecationWarning,
)
dpls = _BACKEND.simulate(net, tstop, dt, n_trials, postproc)
return dpls
if __name__ == "__main__":
print("Hello world!")
Notice how, among other changes, the primary function arguments are easier to read (and diff), and how all quotes are now double-quotes (when it was a mixture before). All of this is, of course, configurable (including the non-PEP8 line length).
We can also double-check that this new file is PEP8 compliant (except for the line length), including no longer having the prior PEP8 violations:
(hc12) [19:33] ~hc [remotes/samadpls/feat/batch_simulation] > flake8 --max-line-length=88 example_format_ruff.py
(hc12) [19:33] ~hc [remotes/samadpls/feat/batch_simulation] > autopep8 --diff --max-line-length=88 example_format_ruff.py
(hc12) [19:33] ~hc [remotes/samadpls/feat/batch_simulation] >
I noticed a particularly good example recently, with the arguments to BatchSimulation. When I was looking at the diff in a recent PR that added some arguments and changed the order of some arguments in BatchSimulate.__init__, it was difficult to read what changes occurred to pre-existing vs new arguments: https://github.com/jonescompneurolab/hnn-core/pull/857/files#diff-f2780322abf4ffe47adb1211d552f3809ea8e151a810b91230a42a945f08104aL19-L27
If we used ruff format as part of the development flow, then this
class BatchSimulate(object):
def __init__(self, set_params, net=None,
tstop=170, dt=0.025, n_trials=1,
save_folder='./sim_results', batch_size=100,
overwrite=True, save_outputs=False, save_dpl=True,
save_spiking=False, save_lfp=False, save_voltages=False,
save_currents=False, save_calcium=False, record_vsec=False,
record_isec=False, postproc=False, clear_cache=False,
summary_func=None):
"""Initialize the BatchSimulate class."""
pass
would become this
class BatchSimulate(object):
def __init__(
self,
set_params,
net=None,
tstop=170,
dt=0.025,
n_trials=1,
save_folder="./sim_results",
batch_size=100,
overwrite=True,
save_outputs=False,
save_dpl=True,
save_spiking=False,
save_lfp=False,
save_voltages=False,
save_currents=False,
save_calcium=False,
record_vsec=False,
record_isec=False,
postproc=False,
clear_cache=False,
summary_func=None,
):
"""Initialize the BatchSimulate class."""
pass
which is both easier to read and to diff. The user making the changes would also not have to worry about improving the formatting by hand; it would work automatically, such as using a make rule.
By now, I hope that I've made it clear how we can both think and communicate about Linting vs Formatting, or at least discuss bug-errors vs style-errors.
More as a point of clarity than anything else, I find the difference between linting vs formatting in the lexicon to be vague in practice. I think this equally applies for how we differentiate "style of code formatting" vs "style of programming". Much of the time, these two things are distinguishable, but in popular style guides, they are often intertwined. More specifically, for PEP8 https://peps.python.org/pep-0008/ , a great portion of it is dedicated to code formatting. However, much of it is not about formatting, but rather how to organize your code in terms of idioms or even just general advice, like https://peps.python.org/pep-0008/#programming-recommendations . For this reason, I think of these recommendations more as "programming style guides" as opposed to "code format style guides". In general, these are also "style guide" choices that are either difficult or impossible to detect violations of using automated programs (for now!). Some other examples of this are many rules in both https://docs.scipy.org/doc/scipy/dev/missing-bits.html#missing-bits and https://google.github.io/styleguide/pyguide.html . My main point is simply that it is one thing to decide on a shared code format style, and another to decide on a shared "programming style guide" of rules such as https://google.github.io/styleguide/pyguide.html#22-imports where you, oftentimes, need to have a human interpret the code to understand if it's even violating things in the first place.
For "programming style guides", as opposed to a "code format style" we may want to begin adoption of one at some point in the future (I don't think it's worth the effort to enforce it). However, I think we would be much better served by only supporting that internally, if at all, and not forcing external contributors to follow a "programming" style guide, since it would increase difficulty of them contributing.