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

Code cleanup for relecov_core #121

Merged
merged 15 commits into from
Jul 23, 2024
Merged

Code cleanup for relecov_core #121

merged 15 commits into from
Jul 23, 2024

Conversation

Shettland
Copy link
Member

This PR addresses several changes to the relecov_core code:

  • Renamed files in relecov_core which had redundant names samples_handling.py -> samples.py
  • Imports in relecov_core and relecov_dashboard have been updated accordingly to these new filenames
  • Changed local from imports to include the whole library name in the import, in order to detect where the imported object come from in the code.
  • All the imported functions have been renamed accordingly
  • Included some #FIXME and #TODO tags in the code with comments to review the code

Copy link
Member

@Daniel-VM Daniel-VM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good. Just few things to accomplish:

  • There are a few missing refactorings of imports that need to be addressed globally in the PR.
  • Local imports should follow a consistent structure accross files by importing the entire file rather than specific components.
  • Follow plotly conventions when importing this library.
  • There are aliases used inconsistently in local imports. We need to decide whether to use them for local imports or not.

relecov_core/api/utils/variants.py Outdated Show resolved Hide resolved
relecov_core/api/views.py Outdated Show resolved Hide resolved
relecov_core/utils/lineage.py Outdated Show resolved Hide resolved
relecov_core/utils/metadata.py Outdated Show resolved Hide resolved
relecov_core/utils/plotly_dash_graphics.py Show resolved Hide resolved
relecov_core/utils/samples_graphics.py Outdated Show resolved Hide resolved
relecov_core/utils/variants.py Outdated Show resolved Hide resolved
relecov_core/views.py Outdated Show resolved Hide resolved
relecov_dashboard/utils/methodology_index.py Outdated Show resolved Hide resolved
@saramonzon
Copy link
Member

I see some renaming left, where p.e "handling" can be removed from the file name

Copy link
Member

@Daniel-VM Daniel-VM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Shettland Shettland merged commit a5ada62 into BU-ISCIII:develop Jul 23, 2024
2 checks passed
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.

3 participants