Skip to content

Fix linting issues #185

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.

Sign up for GitHub

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

Jump to bottom
Merged
stes merged 10 commits into from
Oct 26, 2024
Merged

Fix linting issues #185

stes merged 10 commits into from
Oct 26, 2024

Conversation

@stes
Copy link
Member

@stes stes commented Oct 20, 2024

After merging #167 , this PR fixes all linting issues. This includes:

  • whitespace issues and other easy to fix/autofix issues
  • long lines (especially in docs!)
  • typos in variable names
  • duplicate class definitions (a couple in the allen datasets) which can cause unwanted behavior
  • some unused imports
  • missing variables in some parts of the code not covered by tests yet
@cla-bot cla-bot bot added the CLA signed label Oct 20, 2024
@stes stes mentioned this pull request Oct 20, 2024
Merged
@stes stes self-assigned this Oct 20, 2024
@stes stes added the enhancement New feature or request label Oct 20, 2024
@stes
Copy link
Member Author

stes commented Oct 20, 2024

Seems like my docstring fixes broke some tests. Will fix
@stes stes force-pushed the stes/fix-linting-issues branch from 712c2d9 to 88077ab Compare October 20, 2024 17:40
@stes stes requested a review from MMathisLab October 20, 2024 18:52
@stes
Copy link
Member Author

stes commented Oct 20, 2024

@MMathisLab , this is also finally done! should fix all remaining linting errors in ruff. found some actual errors in the code we missed before before the code in the respective section is not fully covered by tests (should be fixed in a future PR)
@stes stes force-pushed the stes/fix-linting-issues branch from 7ce8453 to a84a940 Compare October 26, 2024 13:54
@stes
Copy link
Member Author

stes commented Oct 26, 2024

@MMathisLab good to go once tests pass?
MMathisLab
MMathisLab approved these changes Oct 26, 2024
View reviewed changes
Copy link
Member

@MMathisLab MMathisLab left a comment

Choose a reason for hiding this comment

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

Did not locally test, but looking over it looks great, thanks @stes !
@stes
Copy link
Member Author

stes commented Oct 26, 2024

Ok sth broke

Will fix later and then merge
@stes stes merged commit 6008052 into main Oct 26, 2024
@stes stes deleted the stes/fix-linting-issues branch October 26, 2024 21:52
@stes stes mentioned this pull request Oct 27, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA signed enhancement New feature or request

3 participants

@stes @MMathisLab