Closed marscher closed 1 year ago
@MarcoGorelli I think this is ready to be reviewed!
Thanks for the review. I've added the requested changes.
Thanks! I'll try this out later but it should be good to go
I just tested the hook locally and I need to pass the "verbose" flag to pre-commit so the errors show up. Also it does not fail, even if the passed notebook contains errors. What am I missing?
Ruff returns a code != 0, in case of an error.
Hey - sorry, just getting back to this now
Also it does not fail, even if the passed notebook contains errors
hmm this doesn't look quite right - could you show an example please?
Also, looks like I need to fix compat with flake8 6.0.0 to get CI to pass, I'll do that separately
Can't reproduce this BTW:
(.venv) marcogorelli@DESKTOP-U8OKFP3:~/nbQA-dev$ nbqa ruff tests/data/notebook_for_testing.ipynb
Found 7 error(s).
tests/data/notebook_for_testing.ipynb:cell_1:1:8: F401 `os` imported but unused
tests/data/notebook_for_testing.ipynb:cell_1:3:8: F401 `glob` imported but unused
tests/data/notebook_for_testing.ipynb:cell_1:5:8: F401 `nbqa` imported but unused
tests/data/notebook_for_testing.ipynb:cell_4:1:1: E402 Module level import not at top of file
tests/data/notebook_for_testing.ipynb:cell_4:1:20: F401 `random.randint` imported but unused
tests/data/notebook_for_testing.ipynb:cell_5:1:1: E402 Module level import not at top of file
tests/data/notebook_for_testing.ipynb:cell_5:2:1: E402 Module level import not at top of file
4 potentially fixable with the --fix option.
(.venv) marcogorelli@DESKTOP-U8OKFP3:~/nbQA-dev$ echo $?
1
I just tested the hook locally and I need to pass the "verbose" flag to pre-commit so the errors show up. Also it does not fail, even if the passed notebook contains errors. What am I missing?
Ruff returns a code != 0, in case of an error.
Still can't reproduce this:
(.venv) marcogorelli@DESKTOP-U8OKFP3:~/tmp$ pre-commit run nbqa-ruff -a
nbqa-ruff................................................................Failed
- hook id: nbqa-ruff
- exit code: 1
notebook_for_testing.ipynb:cell_1:1:8: F401 `os` imported but unused
notebook_for_testing.ipynb:cell_1:3:8: F401 `glob` imported but unused
notebook_for_testing.ipynb:cell_1:5:8: F401 `nbqa` imported but unused
notebook_for_testing.ipynb:cell_4:1:1: E402 Module level import not at top of file
notebook_for_testing.ipynb:cell_4:1:20: F401 `random.randint` imported but unused
notebook_for_testing.ipynb:cell_5:1:1: E402 Module level import not at top of file
notebook_for_testing.ipynb:cell_5:2:1: E402 Module level import not at top of file
Found 7 error(s).
4 potentially fixable with the --fix option.
Let's ship it then, if any bug is reported we can address later
@all-contributors please add @marscher for features
@MarcoGorelli
I couldn't determine any contributions to add, did you specify any contributions? Please make sure to use valid contribution names.
@all-contributors please add @marscher for code
@MarcoGorelli
I've put up a pull request to add @marscher! :tada:
Apologies for the spam, but this is really cool! Thank you for the integration!
Apologies for the spam, but this is really cool! Thank you for the integration!
thanks!
BTW are you signed up to github sponsors? I'd love to sponsor ruff
@MarcoGorelli - Thank you, that's super kind! I'm not signed up yet but I am considering it. I will let you know if I turn it on :)
I'd like an suggestion what a useful test would look like. I believe this tools output could likely change in the future, so the usefulness of strict output checking right now seems overkill. You have to decide.
Fixes #772