Generate PEP8 compliant Python code

[smiths]

The generated Python code is not PEP8 compliant. There aren't too many problems though. Running flake8 finds errors related to the number # leading block comments, line length and whitespace. For instance, the following output is produced when running flake8 on Projectile:

./Calculations.py:1:1: E266 too many leading '#' for block comment
./Calculations.py:6:1: E266 too many leading '#' for block comment
./Calculations.py:10:1: E302 expected 2 blank lines, found 1
./Calculations.py:13:1: E266 too many leading '#' for block comment
./Calculations.py:13:80: E501 line too long (113 > 79 characters)
./Calculations.py:16:80: E501 line too long (102 > 79 characters)
./Calculations.py:17:1: E302 expected 2 blank lines, found 1
./Calculations.py:18:80: E501 line too long (104 > 79 characters)
./Calculations.py:20:1: E266 too many leading '#' for block comment
./Calculations.py:20:80: E501 line too long (151 > 79 characters)
./Calculations.py:22:80: E501 line too long (108 > 79 characters)
./Calculations.py:23:80: E501 line too long (140 > 79 characters)
./Calculations.py:24:1: E302 expected 2 blank lines, found 1
./Calculations.py:27:1: E266 too many leading '#' for block comment
./Calculations.py:30:80: E501 line too long (148 > 79 characters)
./Calculations.py:32:1: E302 expected 2 blank lines, found 1
./Calculations.py:33:59: E203 whitespace before ':'
./Calculations.py:35:26: E203 whitespace before ':'
./Calculations.py:37:9: E203 whitespace before ':'
./Control.py:1:1: E266 too many leading '#' for block comment
./InputParameters.py:1:1: E266 too many leading '#' for block comment
./InputParameters.py:3:80: E501 line too long (158 > 79 characters)
./InputParameters.py:6:1: E266 too many leading '#' for block comment
./InputParameters.py:7:1: E302 expected 2 blank lines, found 1
./InputParameters.py:8:5: E266 too many leading '#' for block comment
./InputParameters.py:8:80: E501 line too long (103 > 79 characters)
./InputParameters.py:13:1: W293 blank line contains whitespace
./InputParameters.py:14:5: E266 too many leading '#' for block comment
./InputParameters.py:25:1: W293 blank line contains whitespace
./InputParameters.py:26:5: E266 too many leading '#' for block comment
./InputParameters.py:28:16: E275 missing whitespace after keyword
./InputParameters.py:28:38: E203 whitespace before ':'
./InputParameters.py:36:16: E275 missing whitespace after keyword
./InputParameters.py:36:66: E203 whitespace before ':'
./InputParameters.py:47:16: E275 missing whitespace after keyword
./InputParameters.py:47:38: E203 whitespace before ':'
./OutputFormat.py:1:1: E266 too many leading '#' for block comment
./OutputFormat.py:4:1: E266 too many leading '#' for block comment
./OutputFormat.py:6:80: E501 line too long (148 > 79 characters)

[peter-michalski]

E266 too many leading '#' for block comment

https://github.com/JacquesCarette/Drasil/blob/84272acccc09574dec70d8d96c6ea994f15f8b22/code/drasil-gool/lib/GOOL/Drasil/LanguageRenderer/PythonRenderer.hs#L763-L768

Updating pyDocCommentStart = pyCommentStart <> pyCommentStart to pyDocCommentStart = pyCommentStart will fix this error.

E203 whitespace before ':'

This is bit more problematic since the issue is not in Python specific code, but in LanguagePolymorphic.hs:

https://github.com/JacquesCarette/Drasil/blob/84272acccc09574dec70d8d96c6ea994f15f8b22/code/drasil-gool/lib/GOOL/Drasil/LanguageRenderer/LanguagePolymorphic.hs#L395-L412

Simply updating the three instances of <+> ifStart to <> ifStart will fix the error in generated Python code, but then create the same opposite problem for other languages, like Java, by removing the space to the left of the curly bracket.

[smiths]

Thank you for the analysis @peter-michalski. I think you should go ahead and fix the too many leading '#' for block comment problem. It looks like an easy fix, and it will reduce the number of flake8 complaints.

For the other problem of the whitespace before : my first thought was that we could configure flake8 to ignore this warning. However, when I look at the code, I agree with flake8. For instance, this code from Projectile in Calculations.py:

def func_s(inParams, epsilon, d_offset):
    if (math.fabs(d_offset / inParams.p_target) < epsilon) :
        return "The target was hit."
    elif (d_offset < 0.0) :
        return "The projectile fell short."
    else :
        return "The projectile went long."

It would look better without the space before :.

The corresponding Java code does look better with the space, as shown below:

public static String func_s(InputParameters inParams, double epsilon, double d_offset) {
        if (Math.abs(d_offset / inParams.p_target) < epsilon) {
            return "The target was hit.";
        }
        else if (d_offset < 0.0) {
            return "The projectile fell short.";
        }
        else {
            return "The projectile went long.";
        }
    }

If we want to keep both, then I guess we have to provide a parameter to LanguagePolymorphic to distinguish languages that want a space from those that don't.

I guess how much we prioritize this depends on how much we want to say that we generate PEP8 compliant code.

[JacquesCarette]

I wonder why pyDocCommentStart = pyCommentStart <> pyCommentStart was defined that way at all? Perhaps this is an older style that is deprecated? This doesn't seem accidental, so I'd like to understand a little more first. [I think we will eventually do the fix that @peter-michalski suggests]

Regarding spacing in code: here I think the 'right' fix is to use <> and then either

1. define ifStart for Java to contain a space (works, but a little hacky)
2. add a (new)optSpace combinator to the GOOL interface and use optSpace ifStart instead of ifStart, where each language will then have to define its own optSpace.

[smiths]

Good question @JacquesCarette about pyDocCommentStart = pyCommentStart <> pyCommentStart. Now that you mention it, I believe this is needed for doxygen. The double number sign (##) at the beginning of a comment block tell Doxygen to parse this area. We could solve the problem by telling flake8 to ignore this error. This can be done with a command line option to flake8, or by adding a configuration file with the same information, as described here. We could also use a different way to tell doxygen about a comment block. From the first link above, I believe we could use """!.

For the space in the code solution, I like the second option.

[smiths]

@harmanpreet-sagar this issue was started, but not finished. Could you please take a crack at it? It would be nice to be able to "brag" that we can generate PEP8 compliant code.

[harmanpreet-sagar]

Once #3643 is merged, all errors relating to E203 whitespace before ':' will have been eliminated from the codebase. The remaining errors for python files in projectile for example would include:

.\projectile_c_p_nol_b_u_v_d\src\python\Calculations.py:1:1: E266 too many leading '#' for block comment
.\projectile_c_p_nol_b_u_v_d\src\python\Calculations.py:7:1: E266 too many leading '#' for block comment
.\projectile_c_p_nol_b_u_v_d\src\python\Calculations.py:11:1: E302 expected 2 blank lines, found 1
.\projectile_c_p_nol_b_u_v_d\src\python\Calculations.py:14:1: E266 too many leading '#' for block comment
.\projectile_c_p_nol_b_u_v_d\src\python\Calculations.py:14:80: E501 line too long (113 > 79 characters)
.\projectile_c_p_nol_b_u_v_d\src\python\Calculations.py:17:80: E501 line too long (102 > 79 characters)
.\projectile_c_p_nol_b_u_v_d\src\python\Calculations.py:18:1: E302 expected 2 blank lines, found 1
.\projectile_c_p_nol_b_u_v_d\src\python\Calculations.py:19:80: E501 line too long (99 > 79 characters)
.\projectile_c_p_nol_b_u_v_d\src\python\Calculations.py:21:1: E266 too many leading '#' for block comment
.\projectile_c_p_nol_b_u_v_d\src\python\Calculations.py:21:80: E501 line too long (151 > 79 characters)
.\projectile_c_p_nol_b_u_v_d\src\python\Calculations.py:23:80: E501 line too long (108 > 79 characters)
.\projectile_c_p_nol_b_u_v_d\src\python\Calculations.py:24:80: E501 line too long (140 > 79 characters)
.\projectile_c_p_nol_b_u_v_d\src\python\Calculations.py:25:1: E302 expected 2 blank lines, found 1
.\projectile_c_p_nol_b_u_v_d\src\python\Calculations.py:28:1: E266 too many leading '#' for block comment
.\projectile_c_p_nol_b_u_v_d\src\python\Calculations.py:31:80: E501 line too long (148 > 79 characters)
.\projectile_c_p_nol_b_u_v_d\src\python\Calculations.py:33:1: E302 expected 2 blank lines, found 1
.\projectile_c_p_nol_b_u_v_d\src\python\Control.py:1:1: E266 too many leading '#' for block comment
.\projectile_c_p_nol_b_u_v_d\src\python\InputParameters.py:1:1: E266 too many leading '#' for block comment
.\projectile_c_p_nol_b_u_v_d\src\python\InputParameters.py:3:80: E501 line too long (158 > 79 characters)
.\projectile_c_p_nol_b_u_v_d\src\python\InputParameters.py:7:1: E266 too many leading '#' for block comment
.\projectile_c_p_nol_b_u_v_d\src\python\InputParameters.py:8:1: E302 expected 2 blank lines, found 1
.\projectile_c_p_nol_b_u_v_d\src\python\InputParameters.py:9:5: E266 too many leading '#' for block comment
.\projectile_c_p_nol_b_u_v_d\src\python\InputParameters.py:9:80: E501 line too long (103 > 79 characters)
.\projectile_c_p_nol_b_u_v_d\src\python\InputParameters.py:14:1: W293 blank line contains whitespace
.\projectile_c_p_nol_b_u_v_d\src\python\InputParameters.py:15:5: E266 too many leading '#' for block comment
.\projectile_c_p_nol_b_u_v_d\src\python\InputParameters.py:26:1: W293 blank line contains whitespace
.\projectile_c_p_nol_b_u_v_d\src\python\InputParameters.py:27:5: E266 too many leading '#' for block comment
.\projectile_c_p_nol_b_u_v_d\src\python\InputParameters.py:29:16: E275 missing whitespace after keyword
.\projectile_c_p_nol_b_u_v_d\src\python\InputParameters.py:37:16: E275 missing whitespace after keyword
.\projectile_c_p_nol_b_u_v_d\src\python\InputParameters.py:48:16: E275 missing whitespace after keyword
.\projectile_c_p_nol_b_u_v_d\src\python\OutputFormat.py:1:1: E266 too many leading '#' for block comment
.\projectile_c_p_nol_b_u_v_d\src\python\OutputFormat.py:5:1: E266 too many leading '#' for block comment
.\projectile_c_p_nol_b_u_v_d\src\python\OutputFormat.py:7:80: E501 line too long (148 > 79 characters)

Another thing to keep in mind when solving this issue is that gooltest is not automatically generated using make pr_ready and hence is not fixed when make stabilize is run. Since gooltest contains Python files, make codegenTest or makecodegenTest_gen must be run to generate the changed code files. Then the stable version of the codefiles within gooltest must be fixed manually.

[smiths]

For E266 we can remove it by adding a .flake8 file to each project. This flake8 config file can tell flake8 to ignore warnings we wish to eliminate.

[harmanpreet-sagar]

#3644 adds the .flake8 config file and ensures that flake8 would ignore the E266 too many leading '#' for block comment warning. The flake8 errors that remain in projectile for example are:

.\projectile_c_p_nol_b_u_v_d\src\python\Calculations.py:11:1: E302 expected 2 blank lines, found 1
.\projectile_c_p_nol_b_u_v_d\src\python\Calculations.py:14:80: E501 line too long (113 > 79 characters)
.\projectile_c_p_nol_b_u_v_d\src\python\Calculations.py:17:80: E501 line too long (102 > 79 characters)
.\projectile_c_p_nol_b_u_v_d\src\python\Calculations.py:18:1: E302 expected 2 blank lines, found 1
.\projectile_c_p_nol_b_u_v_d\src\python\Calculations.py:19:80: E501 line too long (99 > 79 characters)
.\projectile_c_p_nol_b_u_v_d\src\python\Calculations.py:21:80: E501 line too long (151 > 79 characters)
.\projectile_c_p_nol_b_u_v_d\src\python\Calculations.py:23:80: E501 line too long (108 > 79 characters)
.\projectile_c_p_nol_b_u_v_d\src\python\Calculations.py:24:80: E501 line too long (140 > 79 characters)
.\projectile_c_p_nol_b_u_v_d\src\python\Calculations.py:25:1: E302 expected 2 blank lines, found 1
.\projectile_c_p_nol_b_u_v_d\src\python\Calculations.py:31:80: E501 line too long (148 > 79 characters)
.\projectile_c_p_nol_b_u_v_d\src\python\Calculations.py:33:1: E302 expected 2 blank lines, found 1
.\projectile_c_p_nol_b_u_v_d\src\python\InputParameters.py:3:80: E501 line too long (158 > 79 characters)
.\projectile_c_p_nol_b_u_v_d\src\python\InputParameters.py:8:1: E302 expected 2 blank lines, found 1
.\projectile_c_p_nol_b_u_v_d\src\python\InputParameters.py:9:80: E501 line too long (103 > 79 characters)
.\projectile_c_p_nol_b_u_v_d\src\python\InputParameters.py:14:1: W293 blank line contains whitespace
.\projectile_c_p_nol_b_u_v_d\src\python\InputParameters.py:26:1: W293 blank line contains whitespace
.\projectile_c_p_nol_b_u_v_d\src\python\InputParameters.py:29:16: E275 missing whitespace after keyword
.\projectile_c_p_nol_b_u_v_d\src\python\InputParameters.py:37:16: E275 missing whitespace after keyword
.\projectile_c_p_nol_b_u_v_d\src\python\InputParameters.py:48:16: E275 missing whitespace after keyword
.\projectile_c_p_nol_b_u_v_d\src\python\OutputFormat.py:7:80: E501 line too long (148 > 79 characters)

[balacij]

It seems like conventionally, Python users prefer block comments (using """) and the Doxygen manual shows how to use them for Doxygen comments too (using """! to enable special commands). The Python-preferred and Doxygen-advertised way seems to conform to PEP8, so maybe we should switch to generating that code instead of ignoring E266?

Right now, we have: https://github.com/JacquesCarette/Drasil/blob/a3a05381b4f12dc4c1b8cce16fe011628577a413/code/stable/glassbr/src/python/InputConstraints.py#L1-L10

But I propose we change the style of comments to:

"""!
\file InputConstraints.py
\author Nikitha Krithnan and W. Spencer Smith
\brief Provides the function for checking the physical constraints and software constraints on the input
\Generated by Drasil v0.1-alpha
"""

def input_constraints(inParams):
    """!
    \brief Verifies that input values satisfy the physical constraints and software constraints
    \param inParams structure holding the input values
    """
    outfile = open("log.txt", "a")
    print("function input_constraints called with inputs: {", file=outfile)
    print("  inParams = ", end="", file=outfile)

~~Of course, this would mean we close #3644 as it wouldn't be needed anymore. That being said, we should still create a ticket about adding a make lint target to each of the generated examples.~~ Regardless, #3644 should be closed because it only creates a single flake8 config file, but we should be generating that per-python-project instead.

[smiths]

@balacij I agree with your recommendation, both on the change of long comment delimiters and on the point that flake8 configurations should be per python project.

[smiths]

Once we get flake8 working, we could look at pylint. From some ad hoc checks on the Drasil code, it looks like pylint uncovers more areas for investigation than flake8.