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: Move All Imports to the Top of Files #2

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Gfecito
Copy link

@Gfecito Gfecito commented Oct 17, 2024

This PR refactors the existing Python code by moving all imports to the top of each file, following the best practices recommended in the official Python documentation (PEP 8).

This change aims to enhance code maintainability, readability, and may improve performance by preventing redundant imports when functions are called repeatedly.

Key Changes:

  • Consolidated Imports: All imports have been moved to the top of their respective files to comply with Python's standard practices.
  • Adherence to Best Practices: The code now follows the guidelines for import statements, making it more consistent and easier to maintain.
  • Potential Performance Gains: By moving imports out of function definitions, we avoid re-importing modules multiple times during repeated function calls, potentially enhancing runtime efficiency.

Next Steps (Before Merge):

  • Establish Comprehensive Test Bed: Before merging, set up a thorough testing suite to cover all critical paths and ensure that moving imports has not inadvertently caused regressions or other issues.
  • CI Integration: Integrate the test bed into the continuous integration (CI) pipeline to automate testing and enforce code quality standards.

Next Steps (Post Merge):

  • Additional Refactoring Opportunities: As a follow-up, consider reducing the amount of top-level code by abstracting logic into well-defined classes and functions. This can improve maintainability, testability, and scalability of the codebase.

Important Note:

Please ensure that the test bed is in place and passes all critical tests before proceeding with the merge.

@@ -524,7 +523,6 @@ def from_json(
From SmilesClickChem: https://zenodo.org/record/4100676

"""
import pandas as pd
Copy link
Author

Choose a reason for hiding this comment

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

(This was duplicated)

Copy link
Author

Choose a reason for hiding this comment

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

There may be other imports than we can remove, but its outside of the scope of this change.
It'd be prioritary to set the test bed and reduce repetition first, and only then hunt these down.

@Gfecito Gfecito marked this pull request as ready for review October 17, 2024 02:09
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