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

Integrate isort #385

Merged

Conversation

RomiPolaczek
Copy link
Collaborator

@RomiPolaczek RomiPolaczek commented Dec 7, 2024

Goal

Arrange Fuse's code base, and particular the imports in each file.
Related issue: #214

Key files:

  • .pre-commit-config.yaml: Add isort as a pre-commit hook to enforce consistent import order.
  • /.github/workflows/lint.yaml: Added isort into Github actions for automated linting during CI.
  • setup.cfg: Configured isort to use the black profile, ensuring alignment between import sorting and code formatting standards.

@RomiPolaczek RomiPolaczek changed the title Integrate isort for Import Sorting Integrate isort Dec 7, 2024
mosheraboh
mosheraboh previously approved these changes Dec 16, 2024
Copy link
Collaborator

@mosheraboh mosheraboh left a comment

Choose a reason for hiding this comment

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

Hi @RomiPolaczek
Great work!
Approving with one question inline
(making sure that the precommit step both checks and applies fixes)

@@ -25,3 +25,7 @@ repos:
args:
- --remove-empty-cells
- --preserve-cell-outputs
- repo: https://github.com/pycqa/isort
Copy link
Collaborator

Choose a reason for hiding this comment

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

I assume this step checks and applies fixes if necessary, right?
Is the order important? meaning if black should run before or after.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi @mosheraboh,

Thank you for the feedback! 🙌

Yes, the pre-commit step for isort both checks and applies fixes as necessary to ensure imports are properly sorted.
As for the order: It's generally recommended that isort runs before black, as isortorganizes imports and black formats the entire file. This avoids potential conflicts where black might undo some of isort's changes.

I’ve now pushed a commit that updates the order in .pre-commit-config.yaml to ensure isort runs before black.

Copy link
Collaborator

Choose a reason for hiding this comment

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

🙏

Copy link
Collaborator

@mosheraboh mosheraboh left a comment

Choose a reason for hiding this comment

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

Great! Thanks again.

@mosheraboh mosheraboh merged commit 804c185 into BiomedSciAI:master Dec 18, 2024
6 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.

2 participants