search

gchq / coreax

A library for coreset algorithms, written in Jax for fast execution and GPU support.
Apache License 2.0
20 stars 2 forks source link

Add pre-commit hook setup instructions for isort to contributor guidelines #83

Closed tp832944 closed 1 year ago

tp832944 commented 1 year ago

@tp832944 has tested hook and created a script saved on an internal computer. This task is to share the output.

be904619 commented 1 year ago

@gchqdev227 : add comments on additional pre-commit checks which could be added

gchqdev227 commented 1 year ago 
repos:
  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.4.0
    hooks:
      - id: trailing-whitespace  # removes whitespace at the end of lines 
      - id: end-of-file-fixer  # files must end in a single newline
      - id: check-yaml  # tries to load YAML files and fails upon error
      - id: check-added-large-files  # rejects commits which add large files (coverage.xml, for example)
        args: [--enforce-all, --maxkb=100]  # sets the upper limit (might be worth bringing this down)
  - repo: https://github.com/pycqa/isort
    rev: 5.11.5
    hooks:
      - id: isort  # runs isort.  Best to have options in pyproject.toml to reduce redundancy
  - repo: https://github.com/jumanjihouse/pre-commit-hooks
    rev: 3.0.0
    hooks:
      - id: require-ascii  # fail on non-ascii characters – life-saving for .bib files
      - id: forbid-binary  # no binary files, good for .coverage etc
        exclude: docs/source/_static/.*.png  # This repo had a logo, but this may not be relevant yet
  - repo: https://github.com/Lucas-C/pre-commit-hooks
    rev: v1.4.2
    hooks:
      - id: forbid-crlf  # Forbid files containing CRLF end-lines to be committed (VERY important when also working with Windows)
      - id: forbid-tabs  # no tabs, only spaces
  - repo: https://github.com/streetsidesoftware/cspell-cli
    rev: v6.22.0
    hooks:
      - id: cspell  # run a spellcheck (words pulled from cspell.config.yaml)
        exclude: \.gitignore|.*\.properties
  - repo: https://github.com/pycqa/flake8
    rev: 6.0.0
    hooks:
      - id: flake8  # run flake-8
        args: [--max-line-length=150]  # This will be too long for us
        exclude: docs/source/conf.py
  - repo: https://github.com/pycqa/pydocstyle
    rev: 6.3.0
    hooks:
      - id: pydocstyle  # check PEP-257 and other docstring guidance
        additional_dependencies: ["tomli"]
        exclude: examples|tests|conf.py
  - repo: https://github.com/regebro/pyroma
    rev: "4.1"
    hooks:
    - id: pyroma  # ensures that setup file has necessary information
  - repo: https://github.com/nbQA-dev/nbQA
    rev: 1.2.3
    hooks:
      - id: nbqa-isort  # sorts import statements within a Jupyter notebook
        args: [--line-length=120, --fss, --check-only, --diff]  # probably want to remove --check-only here
      - id: nbqa-flake8  # run flake8 within a Jupyter notebook
        args: [--max-line-length=120, "--ignore=E402,W503,F811"]
      - id: nbqa-black  # run black within a Jupyter notebook
  - repo: https://github.com/kynan/nbstripout
    rev: 0.6.1
    hooks:
      - id: nbstripout  # remove output from Jupyter notebooks (may be relevant, I haven't tried this one)
gchqdev227 commented 1 year ago

I've not run Black personally, but this link shows you how to use it as a pre-commit hook.

tp832944 commented 1 year ago

Blocked by #59 because cannot test run Black until this ticket is complete.

tp832944 commented 1 year ago

Defer Black until later but proceed with the rest now.

tp832944 commented 1 year ago

Blocked awaiting IT approval for use of pre-commit package. Requested immediately prior to posting this.

tp832944 commented 1 year ago

IT approval received.

jd750687 commented 1 year ago

Which linter should be used? The above pre-commit uses flake8. There are three main options (in increasing speed, decreasing strictness): pylint, flake8, ruff. There is also mypy or pyright which can be used in combination with others for type checking. flake8 might be a good middle ground.

tp832944 commented 1 year ago

I'd lean towards pylint, as I think strict is wise unless it drives everyone up the wall. The less style checking I have to do in code review, the better.

tp832944 commented 1 year ago

All hooks listed in https://github.com/gchq/coreax/issues/83#issuecomment-1697525860 implemented except: