alchemyst / Skogestad-Python

Python code for "Multivariable Feedback Control"
111 stars 88 forks source link

Addition of Clonedigger Reports, One Clone Removed #299

Closed JPIvan closed 6 years ago

JPIvan commented 6 years ago

Created a folder containing two clonedigger reports, one before and one after my changes to utilsplot.py.

Changes: rga_plot function duplicated code for plotting with plot_type='input' and plot_type='output' parameter removed

JPIvan commented 6 years ago

As a general rule we do not put generated files in the repository as this leads to churn. So please don't check in the clone digger reports.

I haven't worked on anything like this before, so please excuse me if I make further errors of this sort. Clone digger reports removed. The logic for adding them was that I had to create a python 2 environment to generate them as clone digger does not currently support python 3.

PEP8 Standards:

I have made some edits to adhere to the PEP8 style guide.

https://www.python.org/dev/peps/pep-0008/#indentation

my_list = [
    1, 2, 3,
    4, 5, 6,
]
result = some_function_that_takes_arguments(
    'a', 'b', 'c',
    'd', 'e', 'f',
)

Is now the convention followed.


All string formatting performed with % operator replaced with str.format() method as both instances were present. str.format() chosen because of easier implementation of nested parameters in math format strings.

Reduced some line lengths by separating nested function calls.

Blocked code for clarity.

JPIvan commented 6 years ago

I've had the code checked automatically for PEP8 violations. Violations in rga_plot() reported by pycodestlye:

2:1: E302 expected 2 blank lines, found 1 32:80: E501 line too long (136 > 79 characters) 43:1: W293 blank line contains whitespace 45:18: E203 whitespace before ':' 46:16: E231 missing whitespace after ':' 47:16: E231 missing whitespace after ':' 48:20: E231 missing whitespace after ':' 49:21: E231 missing whitespace after ':' 51:17: E203 whitespace before ':' 52:16: E231 missing whitespace after ':' 53:16: E231 missing whitespace after ':' 54:20: E231 missing whitespace after ':' 55:21: E231 missing whitespace after ':' 71:1: W293 blank line contains whitespace 74:1: W293 blank line contains whitespace 76:1: W293 blank line contains whitespace 87:1: W293 blank line contains whitespace 89:80: E501 line too long (81 > 79 characters) 95:80: E501 line too long (80 > 79 characters) 99:1: W293 blank line contains whitespace 101:1: W293 blank line contains whitespace 107:1: W293 blank line contains whitespace 109:1: W293 blank line contains whitespace 113:1: W293 blank line contains whitespace 118:80: E501 line too long (84 > 79 characters)

All addressed.

alchemyst commented 6 years ago

Thanks for your efforts. I know it can be frustrating to do a good thing only to be told you didn't do it quite right. FYI PyCharm will highlight most of these problems and automatically fix them if you go to "Code|Inspect code".

JPIvan commented 6 years ago

No no, this is your codebase, I want to contribute only something that you are completely happy with. I understand the need to be thorough.