Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor visualize module into multiple files #357

Merged
merged 9 commits into from
Sep 7, 2023
Merged

Conversation

btjanaka
Copy link
Member

@btjanaka btjanaka commented Sep 7, 2023

Description

The visualize module has grown to ~1000 lines, and we expect it to get longer as we introduce further functionality. This length is also making it difficult to run tools like Pylint, which consume more time with file size. Thus, this PR breaks the visualize module into multiple smaller files.

The best way to do this while preserving history is to create multiple branches where we make copies of visualize.py and then merge the copies. This has the drawback of adding many commits to master as we cannot squash the commits (that would destroy the history). For more information on this procedure of splitting files while maintaining history, see here.

In addition to the commits which copy visualize.py, this PR also adds a commit that tidies up the copies by removing the duplicated code.

TODO

  • Split methods into separate files
  • Tidy up files, including creating _utils
  • Check that documentation works
  • Split tests into multiple files (alright to not preserve history here)
    • Will NOT do this since we already have visualization

Questions

Status

  • I have read the guidelines in
    CONTRIBUTING.md
  • I have formatted my code using yapf
  • I have tested my code by running pytest
  • I have linted my code with pylint
  • I have added a one-line description of my change to the changelog in
    HISTORY.md
  • This PR is ready to go

@btjanaka btjanaka merged commit 6d209c0 into master Sep 7, 2023
18 checks passed
@btjanaka btjanaka deleted the visualize-refactor branch September 7, 2023 07:18
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