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

Fix import statement ordering using isort #2489

Merged
merged 4 commits into from
Apr 9, 2024

Conversation

pplantinga
Copy link
Collaborator

Adds isort to the linters. This is a nice way to ensure consistency and make imports easier to find.

The initial version of this PR just uses the defaults from isort but if anyone has opinions there are other ways to sort.

Default sorting is alphabetical within 3 sections:

  1. Standard library imports
  2. Third party imports
  3. Internal imports

There are options, such as sort by length, different sections etc. so if anyone wants a different sort, they should speak up.

@pplantinga pplantinga requested review from Gastron, asumagic, mravanelli and Adel-Moumen and removed request for Gastron April 2, 2024 16:53
Copy link
Collaborator

@asumagic asumagic left a comment

Choose a reason for hiding this comment

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

Checked a good subset of the modified files, seems like a good improvement for me. LGTM.

@Adel-Moumen
Copy link
Collaborator

Hello @pplantinga, very nice PR! Thanks a lot. :)

Do you mind updating your PR with the latest commits in the develop branch? Other than that, I think it is ready to be merged :)

@pplantinga pplantinga merged commit f283e88 into speechbrain:develop Apr 9, 2024
4 checks passed
@pplantinga pplantinga deleted the add-isort branch April 9, 2024 16:40
mravanelli added a commit that referenced this pull request Apr 11, 2024
This reverts commit f283e88, reversing
changes made to 3e56fad.
asumagic added a commit to asumagic/speechbrain that referenced this pull request Apr 11, 2024
mravanelli pushed a commit that referenced this pull request Apr 12, 2024
@Adel-Moumen Adel-Moumen mentioned this pull request Apr 30, 2024
13 tasks
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.

None yet

3 participants