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

chore: Adding formatting, linting and type checking #50

Merged
merged 3 commits into from
Jan 18, 2025

Conversation

chanind
Copy link
Contributor

@chanind chanind commented Jan 18, 2025

This PR adds Linting and Formatting with Ruff, and type-checking with Pyright. It also adds some helpers for running these locally via make. Similar to SAELens, all CI checks can be run locally with make check-ci to ensure CI passes.

Some things to note:

  • I mostly added # type: ignore comments to existing type issues rather than trying to fix them unless the fix was very obvious and simple. These can be fixed over time where it makes sense, but probably not a big deal.
  • The file graphing_utils.py has some function calls that are broken and I didn't know what the right way to fix them was. I just disabled type-checking on that file with a TODO to fix it later. Someone who knows that file better should take a look
  • The file testing_notebooks/main_experiments.py seems completely broken, importing things that don't exist and referring to config options that don't exist. I didn't try to fix this and instead just disabled linting and type checking for this file.

It maybe be good to add a PULL_REQUEST_TEMPLATE.md to this repo similar to SAELens that tells contributors to run make check-ci before submitting their PR to make sure everything passes, but I didn't add that as it's probably out of scope for this PR.

fixes #49

@adamkarvonen
Copy link
Owner

Looks great, thanks!

@adamkarvonen adamkarvonen merged commit a0fb5e9 into adamkarvonen:main Jan 18, 2025
4 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.

Proposal: add linting/formatting and type checking
2 participants