mrdbourke / nutrify

Take a photo of food and learn about it.
https://nutrify.app
MIT License
179 stars 33 forks source link

`pre-commit` breaking (not letting commits happen) #9

Closed mrdbourke closed 3 years ago

mrdbourke commented 3 years ago

Tried installing pre-commit (https://pre-commit.com), it looks like it works for some files but breaks others when I try to commit them.

Because the tests don't pass they never commit.

For example:

flake8...................................................................Failed
- hook id: flake8
- exit code: 1

food_image_collector.py:15:80: E501 line too long (97 > 79 characters)
food_image_collector.py:49:80: E501 line too long (80 > 79 characters)
food_image_collector.py:54:80: E501 line too long (82 > 79 characters)
food_image_collector.py:59:80: E501 line too long (123 > 79 characters)
food_image_collector.py:66:80: E501 line too long (80 > 79 characters)
food_image_collector.py:67:80: E501 line too long (81 > 79 characters)
food_image_collector.py:123:80: E501 line too long (114 > 79 characters)
food_image_collector.py:129:80: E501 line too long (106 > 79 characters)
food_image_collector.py:134:80: E501 line too long (84 > 79 characters)
food_image_collector.py:136:80: E501 line too long (102 > 79 characters)
food_image_collector.py:137:80: E501 line too long (84 > 79 characters)
food_image_collector.py:144:80: E501 line too long (91 > 79 characters)

mypy.....................................................................Passed

See the initial setup here: https://github.com/mrdbourke/nutrify/issues/1#issuecomment-916722957

shivan-s commented 3 years ago

Hi Dan,

Oh the joys! Hope you are enjoying pre-commit!

Good news is that flake8 is complaining about line length: https://www.python.org/dev/peps/pep-0008/#maximum-line-length - according to the Python style guide (aka Holiest Pythonic Bible) there shall be only a max line length of 79 characters.

Usually black formats these line lengths, but doesn't for some reason. (Also, as a side note, ensure the python version is correct in the .pre-commit-config.yaml. I think you are using version 3.7 vs I am using 3.8. Oops!

To overcome pre-commit blocking our commits, we could:

  1. Fix every line. a. We go through every line flake8 has flagged as being too long and trim the length b. add our code for staging with git add . and fire another commit.
  2. We configure flake8 to chill out. (https://rednafi.github.io/digressions/python/2020/04/06/python-precommit.html#running-the-linters-as-pre-commit-hooks). Code below is untested... (I've added an arg to increase line length.
    -   repo: https://github.com/pycqa/flake8
    rev: 3.7.9
    hooks:
    -   id: flake8
      args:
        - "--max-line-length=127"
  3. Finally, the last thing we do is tell pre-commit not to run when commit, we use the -n tag.
    git commit -n -m 'ignoring pre-commit lulz'

    Stay Pythonic Dan, Shivan

shivan-s commented 3 years ago

Also, just re-watched the portion of the stream. Nice work with the hack, but I'm so sorry for the frustration it caused!

I noticed you uninstalled pre-commit, but the issue persists the reason is because it is not installed into git hooks.

So the work around is to use the -n tag.

git commit -n -m 'ignore pre-commit darn it!! omgosh!!!'

I know it can be frustrating, but it's a good idea to have since when other contributors come on board, you don't want them to contribute code smells. Also, a flipside is that the pre-commit is working because it blocks commits with bad code (lol)...

Once again, sorry for the frustration. Hoping this clears things up.

mrdbourke commented 3 years ago

Fixed! Added a line length checker in VSCode to make sure the code is the right length: https://stackoverflow.com/a/29972073

79 chars seems a little low though 🤔 , perhaps I'm used to writing long code in notebooks haha

shivan-s commented 3 years ago

Fixed! Added a line length checker in VSCode to make sure the code is the right length: https://stackoverflow.com/a/29972073

79 chars seems a little low though thinking , perhaps I'm used to writing long code in notebooks haha

Well done! It is low. I guess it's so you can have code one side of the screen and documentation/stack overflow on the other lol!

But really it is do with the punch card era: https://en.wikipedia.org/wiki/Computer_programming_in_the_punched_card_era#Punched_cards. Traditional punch cards being 80 chars long (0-79).