Hidden tips to write flake8 and black compatible code

[sebastien-lenard]

Hi, this discussion to share tips that are not so obvious to write flake8 and black compatible code for landlab.

1. Well to write black compatible code, it's easy since you just have to run black on your modified files. It automatically format files. More on black current and future style format: https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html

2. flake8 is a more capricious person. Again you run them on your modified files and they offer you a splendid report of several lines. You can have severe rants with Flaky, because Flaky don't take in charge code modification, probably because in some cases, modification of ambiguously written codes (especially with indents) can alter what you initially want your code to do. More on Flaky's big words: https://flake8.pycqa.org/en/latest/user/error-codes.html

[sebastien-lenard]

How to break long statements in python, including if statements

I will begin by this tricky error when writing this bit of code :

my_variable_has_a_so_long_and_incredible_name = 1
no_mine_is_much_longer_than_yours_as_you_can_see = 2
c = 0
if my_variable_has_a_so_long_and_incredible_name == 1 \
    and no_mine_is_much_longer_than_yours_as_you_can_see == 2:
    c = 1

which makes Flaky blurp: setup.py:61:5: E125 continuation line with same indent as next logical line

You can also try as an alternative, and let's simply the variable names for clarity:

a = 1
b = 2
c = 0
if a == 1 and \
    b == 2:
    c = 1

Flaky, again, blurps.

Let's try the following, since according to PEP8, long lines should be placed in parentheses. When using parentheses, the lines can be broken up without using backslashes. You should also try to put the line break before boolean operators.

a = 1
b = 2
c = 0
if (a == 1
    and b == 2):
    c = 1

Flaky blurps (at least, they don't vomit)
Actually, you also need to get the next logical line having a different indentation to your code block.

Solution to make Flaky not blurping is:

a = 1
b = 2
c = 0
if (a == 1 and
        b == 2):
    c = 1

[mcflugen]

@sebastien-lenard The preferred method for line continuation is, as you say, to use parenthesis. Your first example would be,

my_variable_has_a_so_long_and_incredible_name = 1
no_mine_is_much_longer_than_yours_as_you_can_see = 2
c = 0
if (
    my_variable_has_a_so_long_and_incredible_name == 1
    and no_mine_is_much_longer_than_yours_as_you_can_see == 2
):
    c = 1

Your second example should just be on a single line,

a = 1
b = 2
c = 0
if a == 1 and b == 2:
    c = 1

This is how black would reformat it and should satisfy flake8.

[sebastien-lenard]

How to write tests in docstrings which respect the 79 character limit imposed by flake8

When you write your unit tests in your docstrings of your methods, sometimes, the return expected make more than 79 characters. For instance this test:

        >>> g.at_node["flow__upstream_node_order"]
        array([ 4,  8,  7,  9, 16, 17, 23, 10,  5, 14, 13, 15, 21, 22, 27, 26, 20,
            19, 25, 18, 11, 24, 29, 30, 12,  6, 28, 32, 33, 34, 35, 36, 31,
            0, 1,  2,  3])

But actually, it is the value returned that pytest will consider and not the string written. So you can shorten your line, with the same result, this way:

        >>> g.at_node["flow__upstream_node_order"]
        array([ 4,  8,  7,  9, 16, 17, 23, 10,  5, 14, 13, 15, 21, 22, 27, 
             26, 20,
            19, 25, 18, 11, 24, 29, 30, 12,  6, 28, 32, 33, 34, 35, 36, 31,
            0, 1,  2,  3])

[mcflugen]

it is the value returned that pytest will consider and not the string written

This isn't quite right. It really is the printed string that pytest is looking at. In your example the second case passes because we set the NORMALIZE_WHITESPACE option (this is set in config.cfg). With this option turned on, pytest matches one whitespace character with one or more whitespace characters.

Even with this option on, the following test would (possibly suprisingly) fail,

>>> g.at_node["flow__upstream_node_order"]
array([4,  8,  7,  9, 16, 17, 23, 10,  5, 14, 13, 15, 21, 22, 27, 
        26, 20,
        19, 25, 18, 11, 24, 29, 30, 12,  6, 28, 32, 33, 34, 35, 36, 31,
        0, 1,  2,  3])

Notice the lack of a space before the first 4 (i.e. array([4 vs. array([ 4).

In setup.cfg we set flake8's maximum line length to 88 characters, not the default 79. Maybe you are using a different linter?

[sebastien-lenard]

You're right @mcflugen, array([4 fails.

I'm just launching flake8 + name of file. So I guess it doesn't use the setup.cfg file. It might be relevant to add this latter info there:
https://landlab.readthedocs.io/en/master/development/practices/style_conventions.html

[sebastien-lenard]

Reformatting of np.arrays written over 2 lines into arrays written over tens of lines within test_xxx.py files using black

It occurs that my array on two lines:

np.array([1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0,
	1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0])

is formatted by black into an array of 29 lines ... not sure that makes the code more readable ...

np.array(
            [
                1.0,
                1.0,
                1.0,
                1.0,
                1.0,
                1.0,
                1.0,
                1.0,
                1.0,
                1.0,
                1.0,
                1.0,
                1.0,
                1.0,
                1.0,
                1.0,
                1.0,
                1.0,
                1.0,
                1.0,
                1.0,
                1.0,
                1.0,
                1.0,
                1.0,
            ]
        ),

Result: my initial test files was at +/- 500 lines passed to a size of 3,000 lines !

[sebastien-lenard]

ok if the test is for testing the result of a method.

But if the test if for testing the attributes of an object, I think that making comparison with a stored version of the expected object is clearer. We just have to store an expected object with pickle.dump (for instance) and the testing process would be this way:
(1) load my expected object with pickle.load
(2) instantiate the object to test
(3) make a loop on the attributes to test, and for each attribute make the assert between the values of the expected object and the instantiated object.

Isn't it clearer for objects having tens of attributes?

[sebastien-lenard]

@mcflugen I tested this way of doing:

expected = np.array(
    [
        [1, 2, 3, 4, 5],
        [6, 7, 8, 9, 10],
        [11, 12, 13, 14, 15],
        [16, 17, 18, 19, 20],
        [21, 22, 23, 24, 25, 26],
    ]
).flatten()

But comparison with a 1D array simply doesn't work, because .flatten() produce this kind of thing:

array([list([1, 2, 3, 4, 5]), list([6, 7, 8, 9, 10]),
       list([11, 12, 13, 14, 15]), list([16, 17, 18, 19, 20]),
       list([21, 22, 23, 24, 25, 26])], dtype=object)

which is not a 1D array ...

NB: I took a very simple sequence of 1,2, ... but it's a similar problem for sequences like the following, on any type of grid:

assert_array_almost_equal(g.at_node["depression_free__elevation"],
        np.array([ 5.66743143, 8.53977988, 6.45357199, 4.11156813, 4.68031945,
            8.21361221, 5.57024205, 7.8622079 , 4.68031945, 2.47630211,
            4.42145537, 8.96951584, 5.57024205, 5.28644224, 2.75982042,
            8.74862188, 5.57024205, 8.05920356, 5.28644224, 4.31320515,
            3.49732191, 5.16016994, 3.69042619, 4.72153783, 1.93599482]))

[mcflugen]

@sebastien-lenard Oh, right. Yes, that wouldn't work because the arrays aren't all the same length (that was a typo). It would work for raster grids where each row has the same number of columns, though.

For an unequal number of values per line, another option could be,

expected = np.array(
    [1, 2, 3, 4, 5]
    + [6, 7, 8, 9, 10]
    + [11, 12, 13, 14, 15]
    + [16, 17, 18, 19, 20]
    + [21, 22, 23, 24, 25, 26]
)

For equal number of rows per line, the following will work,

expected = np.array(
    [
        [1, 2, 3, 4, 5],
        [6, 7, 8, 9, 10],
        [11, 12, 13, 14, 15],
        [16, 17, 18, 19, 20],
        [21, 22, 23, 24, 25],
    ]
).flatten()

[mcflugen]

For your more complicated case, it could be,

assert_array_almost_equal(
    g.at_node["depression_free__elevation"],
    np.array(
        [5.66743143, 8.53977988, 6.45357199, 4.11156813, 4.68031945]
        + [8.21361221, 5.57024205, 7.8622079, 4.68031945, 2.47630211]
        + [4.42145537, 8.96951584, 5.57024205, 5.28644224, 2.75982042]
        + [8.74862188, 5.57024205, 8.05920356, 5.28644224, 4.31320515]
        + [3.49732191, 5.16016994, 3.69042619, 4.72153783, 1.93599482]
    ),
)

[sebastien-lenard]

Thanks. This looks definitely a clean solution. I wrote a little python script below to make the transformations on 1-D and 2-D arrays that cover several, if anybody needs it. The script saves the initial version of the file under the name filename.old.

You have to call the script with the path/filename, e.g.

python myscript.py path/filename.py

For proper working, the lines of 1-D arrays must not contain more than 88 lines and must be cut this way:

        np.array([[0, 3, 7, 11, 14, 18, 24, 30, 36, 40, 44, 50, 56, 62, 68, 72, 75, 81,
                    135, 139, 143, 149, 155, 161, 165, 168, 172, 176, 179]]))

The lines of 2-D arrays must not contain more than 88 characters and must be this way:

        np.array([[0, 3, 7, 11, 14, 18, 24, 30, 36, 40, 44, 50, 56, 62, 68, 72, 75, 81,
                   87, 93, 99, 105, 108, 112, 118, 124, 130, 136, 140, 144, 150, 156,
                   162, 166, 169, 173, 177], [2, 6, 10, 13, 17, 23, 29, 35, 39, 43, 49,
                    55, 61, 67, 71, 74, 80, 86, 92, 98, 104, 107, 111, 117, 123, 129,
                    135, 139, 143, 149, 155, 161, 165, 168, 172, 176, 179]]))

Script:

import sys, re, os
fname = sys.argv[1]
with open(fname, "r") as f:
    code = f.read()
os.rename(fname, fname + ".old")

code = re.sub(', +', ', ', code)
code = re.sub(' +,', ',', code)
code = re.sub(' +\]', ']', code)
code = re.sub('\[ +', '[', code)
code = re.sub("([0-9\.]),\n", r"\1]\n", code)
code = re.sub("  ([-]?[0-9])", r"  +[\1", code)
code = re.sub("  ([0-9])", r"  +[\1", code)

with open(fname, "w") as f:
    f.write(code)