How to satisfy CodeClimate

One part of code review that people can find frustrating (especially first time committers) is how to satisfy CodeClimate.

First up, let me address why this is important. Readability of code is hugely important, especially when diving into a large project for the first time. Let's be honest, it's never going to be possible to open up a random PyCBC module and understand all the code first time. However, if everything is laid out according to a standard - followed across [hopefully] all python projects - it helps to guide your eye and give you a better chance to follow what is going on ... If you can make changes to improve readability please do feel free to propose that!

I let python itself explain why this is important in their words: https://www.python.org/dev/peps/pep-0008/

ARRRGGHHH!! How do I satisfy this stupid thing?

This is perhaps the biggest question/problem with this. Some requested changes are clear (e.g. don't use single-character variable names, use something self-descriptive… Or use two new-lines here). But indents and multi-line wrapping can be confusing.

We do have a standard for this. Where it is unclear what to do, follow these rules: https://black.readthedocs.io/en/stable/

Luckily this can be done automatically. Let's say you have a nasty block like this:

       chi = lalsimulation.SimInspiralTaylorF2ReducedSpinComputeChi(
                          m1,m2,s1z,s2z)
       time_length = lalsimulation.SimInspiralTaylorF2ReducedSpinChirpTime(
                          f_low, m1 * lal.MSUN_SI, m2 * lal.MSUN_SI, chi, -1)              

which CodeClimate will complain about. How to make this nice.

Simply install black. This will need to be done in a python3 virtual environment. An example of creating such an environment and then installing black in it:

mkdir virtualenvs
cd virtualenvs
virtualenv -p python3 pycbc_python3
source pycbc_python3/bin/activate
pip install black

Now you have a python3 virtualenv with black installed in virtualenvs/pycbc_python3/bin/activate

Now let's fix that code. So to do this I create a tiny python file containing the problematic code. It doesn't have to work, but must not have syntax errors. So I create a test.py in an empty directory containing:

for _ in [1]:
    for _ in [1]:
       chi = lalsimulation.SimInspiralTaylorF2ReducedSpinComputeChi(
                          m1,m2,s1z,s2z)
       time_length = lalsimulation.SimInspiralTaylorF2ReducedSpinChirpTime(
                          f_low, m1 * lal.MSUN_SI, m2 * lal.MSUN_SI, chi, -1)              

Then I run:

black test.py
cat test.py

Then I see what this should look like:

for _ in [1]:
    for _ in [1]:
        chi = lalsimulation.SimInspiralTaylorF2ReducedSpinComputeChi(
            m1, m2, s1z, s2z
        )
        time_length = lalsimulation.SimInspiralTaylorF2ReducedSpinChirpTime(
            f_low, m1 * lal.MSUN_SI, m2 * lal.MSUN_SI, chi, -1
        )

On very rare occasions black will not be able to obey the 79-characters-on-a-line-rule. If that is the case after doing this it is fine to overrule CodeClimate… Or try using shorter variable names. While in this example the 79-character limit is obeyed, so there's no problem, you could do:

        lalsimchifunc = lalsimulation.SimInspiralTaylorF2ReducedSpinComputeChi
        lalsimtimefunc = lalsimulation.SimInspiralTaylorF2ReducedSpinChirpTime
        chi = lalsimchifunc(m1, m2, s1z, s2z)
        time_length = lalsimtimefunc(
            f_low, m1 * lal.MSUN_SI, m2 * lal.MSUN_SI, chi, -1
        )

In this particular example this final block is probably less readable than what you got immediately with black.