Skip to content

Conversation

@Lil-Nugs
Copy link

@Lil-Nugs Lil-Nugs commented Apr 30, 2021

Changes I made

  • Added standard python .gitignore along with ignoring the .vscode folder
  • Added @functools.wraps(func) to the output_decorator function
    • This preserves the original function's information, such as the name, docstrings, etc
  • Added a constructor to Test 1 Procedures to allow initialization of test 1 procedures, with the following arguments
    • The general ledger detail dataframe: general_ledger_df
    • The log file dataframe: log_file_df
    • Output folder string: Output_Folder
      • Automatically creates folder if not present
  • Renamed general ledger & log file variables to follow PEP 8 guidelines:
    • GL_Detail_MMMMYYDD_MMMMYYDD -> general_ledger_df
    • Log_File_MMMMYYDD_MMMMYYDD -> log_file_df
  • Refactored some lines to be < 80 characters long in accordance to PEP 8
  • Since there is now a constructor, you can initialize a tester and use it like this:
    • tester = Test_1_Procedures(general_ledger_df, log_file_df)
    • tester.check_for_gaps_in_JE_ID()

Explanations:

Q: Why change GL_Detail_MMMMYYDD_MMMMYYDD to general_ledger_df?

  • The original variable name was very long and made reading the code kind of difficult. The MMMMYYDD doesn't seem to be useful as part of the variable name, and explicitly spelling out general ledger will help keep code readable for non-accountants. Adding df also indicates that it should be a dataframe.

Planned Future Changes

  • Refactor all lines to be less than 80 characters long
  • Noticed the comparison_of_amounts_of_GL_and_log_file doesn't have variables output_file or IN_LOG_not_in_GL
    • Also not used in the jupyter notebook, needs to be fixed
  • Refactor other variable names to be in accordance with PEP 8 (lowercase and snake_case)
  • Refactor pandas code to use method chaining

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant