Code quality guidelines

[chrisconlan]

Here are some things I noticed while reviewing code that should be made rules.

• All function names should be verbs.
• A leading underscore e.g. _some_function can denote a private method, meaning it won't be imported from from qttk import * imports and users know that it is not intended for them.
• Use an indent of 4 spaces
• The @time_this decorator should not wrap functions that will be imported by users. Write your code accordingly.
• Class definitions are title case e.g. MyClassName, while function names are snake case e.g. my_function_name. Variable names are also snake case e.g. my_variable_name.
• Input arguments to functions should not be modified within the function call. This is especially important when dealing with data frames.
• Don't assume you know the user's working directory. Always establish a path to external files relative to the script or module that is importing them, e.g. os.path.join(os.path.dirname(__file__), '..', 'my_data.csv')
• Python filenames should all be snakecase and should not contain numbers.
• We're programming clear and clean code, where the code itself serves as documentation. Think about the end-user's experience of exploring the source code when organizing the repo.
• Use assert statements if __name__ == '__main__': clauses to test code within the same file the code is written.
• Regarding the above, no files should have the word test in them, because that implies adherence to a different testing framework. It is also inappropriate for users to import files with the word test in them.
• Keep it DRY (don't repeat yourself). There should be a single source of truth for each distinct functionality in the repo. If there are multiple versions of a function, which we expect due to our profiling work, it should clear which one is optimal and which one is recommended for production use.
• Date-indexed pandas objects are the backbone data structure of our project.
  • If a function takes a pandas object in, it should generally return a pandas object with the same index. Avoid unnecessarily manipulating the index of the input.
  • Don't return unnecessarily complex objects. Return a pd.Series if possible. Return a pd.DataFrame if necessary.
  • Name the series when helpful. Name the columns of a data frame when helpful.
    • e.g. my_series.name = 'rsi'
    • e.g. my_df.columns = ['a', 'b', 'c']

[emican]

What do you think of another convention: separating the presentation layer (graphs) from modules with calculations? price_crossover.py is an example of this convention and imports rsi and bollinger indicators, _plot is an internal function and the main entry point is where the a graph is created. I struggled to find a way to add tests to bollinger_2.py because graphs are created in main.

[chrisconlan]

That makes sense. I would also like to generalize the charts around the “overlays” vs “indicators” concept.

[emican]

Please allow me to propose this convention for consideration: single purpose modules.

Need:
Allow people to focus on the algorithm iterations when reading modules.

Examples:
Simple moving average was mixed in with cumulative moving averages in cumulative_moving_average.py (formally testma.py)

cumulative_moving_average.py (formally testma.py) has logic to load validation data, we may want to add a new module to utils/data_validation.py and relocate the logic

[joe-wojniak]

yep, that makes sense.

…

On Sat, Jan 30, 2021 at 3:27 PM emican ***@***.***> wrote: Please allow me to propose this convention for your consideration: single purpose modules. Need: Allow people to focus on the algorithm iterations when reading the modules. Examples: Simple moving average is deserving of its own module and it was mixed in with cumulative moving averages in cumulative_moving_average.py (formally testma.py) cumulative_moving_average.py (formally testma.py) has logic to load validation data, we may want to add a new module to utils/data_validation.py and relocate the logic — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#10 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEAV4CDVAFYY2UVXCGPMFSTS4SBVJANCNFSM4WSQTBXQ> .

-- -Joe Wojniak *CONFIDENTIALITY* NOTICE: The contents of this *email* message and any attachments are intended solely for the addressee(s) and may contain *confidential *and/or privileged *information* and may be legally protected from disclosure.

[joe-wojniak]

I'm not sure how much more code refactoring I'll be doing this weekend. But, I think single-purpose modules make a lot of sense.

…

-Joe W.

On Sat, Jan 30, 2021 at 3:57 PM Joe Wojniak ***@***.***> wrote: yep, that makes sense. On Sat, Jan 30, 2021 at 3:27 PM emican ***@***.***> wrote: > Please allow me to propose this convention for your consideration: single > purpose modules. > > Need: > Allow people to focus on the algorithm iterations when reading the > modules. > > Examples: > Simple moving average is deserving of its own module and it was mixed in > with cumulative moving averages in cumulative_moving_average.py (formally > testma.py) > > cumulative_moving_average.py (formally testma.py) has logic to load > validation data, we may want to add a new module to > utils/data_validation.py and relocate the logic > > — > You are receiving this because you were assigned. > Reply to this email directly, view it on GitHub > <#10 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AEAV4CDVAFYY2UVXCGPMFSTS4SBVJANCNFSM4WSQTBXQ> > . > -- -Joe Wojniak *CONFIDENTIALITY* NOTICE: The contents of this *email* message and any attachments are intended solely for the addressee(s) and may contain *confidential *and/or privileged *information* and may be legally protected from disclosure.

-- -Joe Wojniak *CONFIDENTIALITY* NOTICE: The contents of this *email* message and any attachments are intended solely for the addressee(s) and may contain *confidential *and/or privileged *information* and may be legally protected from disclosure.

[emican]

No worries, I can put the changes in one commit so we can easily revert. Next week at the office will be busy so I'm trying to do as much as possible now.

[chrisconlan]

Good idea Eric