johko / computer-vision-course

This repo is the homebase of a community driven course on Computer Vision with Neural Networks. Feel free to join us on the Hugging Face discord: hf.co/join/discord
MIT License
389 stars 126 forks source link

chore: add linters #17

Closed wonhyeongseo closed 9 months ago

wonhyeongseo commented 9 months ago

Fixes https://github.com/johko/computer-vision-course/issues/16

This PR integrates black and ruff as pre-commit hooks to improve our code quality and consistency. With these hooks in place, every code change will be automatically formatted according to the standards set by black and validated by ruff before commits.

Changes

  1. Added black as a pre-commit hook.

    • Ensures consistent code formatting across the codebase.
    • Adheres to the PEP 8 styling guide with minor modifications for readability.
  2. Added ruff as a pre-commit hook.

    • Provides linting checks and highlights potential issues in the code.
    • Helps maintain clean and maintainable code practices.

Benefits

How to Test

  1. Check out this branch.
  2. Install pre-commit: pip install pre-commit.
  3. Set up the git hook scripts: pre-commit install.
  4. Make changes to any .py or .ipynb file.
  5. Try committing your changes. The pre-commit hooks should run black and ruff.

Feedback and suggestions are welcome!

Thank you.

wonhyeongseo commented 9 months ago

Dear @johko and @lunarflu ,

I've added black and ruff as pre-commit hooks. Should we also integrate them as GitHub Actions for added assurance on PRs? Would appreciate your input on the value of these linters as GitHub Actions given potential cost implications.

Thanks for your guidance.

Best, Won

johko commented 9 months ago

One general question @wonhyeongseo : We currently don't have any config files for black and ruff (or a pyproject.toml to cover both). Is that a problem or will it just run with all the default settings now?

lunarflu commented 9 months ago

Dear @johko and @lunarflu ,

I've added black and ruff as pre-commit hooks. Should we also integrate them as GitHub Actions for added assurance on PRs? Would appreciate your input on the value of these linters as GitHub Actions given potential cost implications.

Thanks for your guidance.

Best, Won

Thanks for bringing up! If I understand correctly, it's free for public repos + some small amount of usage, and is another layer of redundance. If so, SGTM, and we can remove if needed.

johko commented 9 months ago

Sorry, forgot to give feedback regarding actions - I also think that it makes sense (and hopefully will be free for us). But you can do this in a separate PR. I think this one is good to merge