Add precommit config - Githubissues

aicoe-aiops / project-template

this is a template to use for new data science projects in the aiops group

Other

8 stars 21 forks source link

Add precommit config #22

Closed tumido closed 4 years ago

tumido commented 4 years ago

Pull precommit config from thoth-template https://github.com/thoth-station/template-project/blob/master/.pre-commit-config.yaml
Add hooks for llinting and checking Jupyter Notebooks:
- Flake8 for notebooks - flake8-nb: https://github.com/s-weigand/flake8-nb
- Black for notebooks - black-nb: https://github.com/tomcatling/black-nb

These 2 hooks above should in theory sufficiently replace the original flake8 and black, because they are executing them anyways, but it didn't work for me - it didn't fix the issues in original "just python" files.

And as a bonus, reformat and fix found violations in the template, so precommit is passing.

sesheta commented 4 years ago

Pre-Commit Test failed! Click here

``` [INFO] Initializing environment for git://github.com/Lucas-C/pre-commit-hooks. [INFO] Initializing environment for git://github.com/pre-commit/pre-commit-hooks. [INFO] Initializing environment for git://github.com/pycqa/pydocstyle.git. [INFO] Initializing environment for https://github.com/pre-commit/pre-commit-hooks. [INFO] Initializing environment for https://github.com/pre-commit/mirrors-mypy. [INFO] Initializing environment for https://github.com/psf/black. [INFO] Initializing environment for https://github.com/tomcatling/black-nb. [INFO] Initializing environment for https://gitlab.com/PyCQA/flake8. [INFO] Initializing environment for https://gitlab.com/PyCQA/flake8:pep8-naming. [INFO] Initializing environment for https://github.com/s-weigand/flake8-nb. An error has occurred: InvalidManifestError: ==> File /tekton/home/.cache/pre-commit/repoumiwvlzu/.pre-commit-hooks.yaml ==> At Hook(id='flake8-nb') ==> At key: types ==> At index 1 =====> Type tag 'jupyter' is not recognized. Try upgrading identify and pre-commit? Check the log at /tekton/home/.cache/pre-commit/pre-commit.log ```

tumido commented 4 years ago

@harshad16 can you please help me with the precommit error? I've tried installing a new hook to provide linting for Jupyter notebooks, but sesheta doesn't like it for some reason. Works just fine locally.

harshad16 commented 4 years ago

@harshad16 can you please help me with the precommit error? I've tried installing a new hook to provide linting for Jupyter notebooks, but sesheta doesn't like it for some reason. Works just fine locally.

@tumido surely, will take a look at this and get back to you on this.

s-weigand commented 4 years ago

@tumido flake8-nb author here, the hook supports *.py AND *.ipynb since version 0.2.3, hope like that it works. 😄 Also, you might want to run pre-commit autoupdate, to get all hooks up to date. 😉

As for the error, the old hook used jupyter as identifier, which files should be run, which is only supported by identify>=1.4.20. To support both *.py and *.ipynb files I couldn't use types in the hook since the logic combines them with an AND, so after you update the revision it should all work fine.

tumido commented 4 years ago

@s-weigand love it! It works flawlessly. :raised_hands: You sir, saved us some time (and electricity) on each CI run now... What a simple change, but what difference it can make.

Thank you for your comment, I wouldn't notice your release for a bit. :smile: And thank you for flake8_nb. It's really handy I must say. :+1:

tumido commented 4 years ago

@durandom wdyt? merge/nomerge that's the question of the day. :smile:

s-weigand commented 4 years ago

@tumido You are welcome and I'm glad my tool is used by someone besides me 😋