nbQA-dev / nbQA

Run ruff, isort, pyupgrade, mypy, pylint, flake8, and more on Jupyter Notebooks
https://nbqa.readthedocs.io/en/latest/index.html
MIT License
1.04k stars 41 forks source link

black being pinned to an older version causes a crash #716

Closed chamilad closed 2 years ago

chamilad commented 2 years ago

There was a breaking change recently in the click library which is a dependency in black. black has updated the dependency version and released a new version, however because black is pinned to an older version, this fix isn't getting propagated. For the moment, we can workaround this by pinning click to an older version, but I think nbqa-black itself can fix this by moving to the newest black version.

I checked the commit that introduced the version pin, but I couldn't figure out the reason for doing so.

Refer to the upstream issue for more details.

chamilad commented 2 years ago

Okay, I've made a mistake. It's not because of black version pinning in the requirements-dev.txt file, it's because of the precommit config. I'll try and send a PR for this update.

MarcoGorelli commented 2 years ago

Hi @chamilad ,

Thanks for the issue - this only affects local nbQA development, right? For users of the library, black isn't pinned (nor is it a required dependency)

chamilad commented 2 years ago

Hi @MarcoGorelli,

Sorry, my issue description is wrong. It affects the library use, at least in my case. I'm guessing it's because of how precommit config works, because I even tried installing Black and Click with pinned latest versions before installing nbqa-black library without success. I couldn't dig deep into this, but I'm guessing precommit uses some kind of venv setup with the precommit config versions of the dependencies installed.

The error is reproduced in a Github pipeline, so I couldn't get into the file system to investigate in depth.

MarcoGorelli commented 2 years ago

Can you show your precommit config file please?

s-weigand commented 2 years ago

@chamilad black fixed this issue by now. The hook from nbqa does not pin the back version https://github.com/nbQA-dev/nbQA/blob/b77ee1424b4ef11878c99ebd8a701ed442fa0aa8/.pre-commit-hooks.yaml#L8-L15 But pre-commit caches the venvs it uses to run the hooks based on the values in your config. Your problem most likely is that the cached venv uses a broken combination of back and click (also had this issue lately). You could just pin the version of black==22.3.0 in the additional_dependencies section of your pre-commit-config.yaml, which would create a new venv due to the changed config. Alternatively, if black isn't pinned in your config you can run pre-commit clean which will delete all cached venvs.

@MarcoGorelli The possibility to create such a broken venv with the default nbqa hook was less than 1 day (breaking click release, fixing black release). The only possibility to end up with a broken pre-commit config now is if users pin an older black version in their additional_dependencies, in which case there is nothing that could be done from the side of nbqa since the hooks additional_dependencies section will be overwritten. Maybe just rename this issue and pin it for a month or so?

chamilad commented 2 years ago

Hi @MarcoGorelli , @s-weigand ,

Yeah, I don't know how to say this without sounding stupid, but you're right. I have jumped the gun on the dependency.

Turns out our Workflow copies over a precommit config from a different repo, which had black pinned to an older version as an additional_dependencies entry.

image

Thanks for your support in getting this cleared! Should I keep the issue open as it's pinned? If not, please close it.

MarcoGorelli commented 2 years ago

No worries, thanks for the report!

I'll close then but will keep pinned for a bit

dnoliver commented 2 years ago

Hi, run into an issue while testing nbqa black for the first time today. Want to check first if it is related to this one.

I was trying to run this tools against a notebook I have, my setup is a windows laptop with docker, and I pulled python:3.6 from dockerhub, started an interactive container with my code mounted as a volume.

Then, created a virtual environment and installed nbqa as:

python3.6 -m venv .venv
python3.6 -m pip install -U nbqa
nbqa black tiny_yolo_v3.ipynb
python3.6 -m pip install black

In line 3 above, nbqa complained that the "black" command was not found. I thought those tools described in https://nbqa.readthedocs.io/en/latest/examples.html came pre-packaged with nbqa :), so I installed black with pip. Then trying to run it, gave me this error (the traces are shortened):

(.venv) root@347d11468761:~/tiny-yolo-v3-python# nbqa black tiny_yolo_v3.ipynb
Traceback (most recent call last):
  File "/tiny-yolo-v3-python/.venv/bin/nbqa", line 11, in <module>
    sys.exit(main())
  File "/tiny-yolo-v3-python/.venv/lib/python3.6/site-packages/nbqa/__main__.py", 
line 708, in main
    return _main(cli_args, configs)
  File "/tiny-yolo-v3-python/.venv/lib/python3.6/site-packages/nbqa/__main__.py", 
line 614, in _main
    for key, i in nb_to_tmp_mapping.items()
  File "/tiny-yolo-v3-python/.venv/lib/python3.6/site-packages/nbqa/__main__.py", 
line 267, in _run_command
    env=my_env,
  File "/usr/local/lib/python3.6/subprocess.py", line 423, in run
    with Popen(*popenargs, **kwargs) as process:
TypeError: __init__() got an unexpected keyword argument 'capture_output'

And my black version was:

(.venv) root@347d11468761:~//tiny-yolo-v3-python# black --version
black, 22.6.0 (compiled: no)
Python (CPython) 3.6.15
(.venv)

~I will try to install black 22.3.0 and run again~ Same error with black==22.3.0

UPDATE: my bad, this error is related to Python 3.6. https://stackoverflow.com/a/53209196 explains the reason.

Using a python:3.7 as the container image, got the thing working:

nbqa black tiny_yolo_v3.ipynb
reformatted tiny_yolo_v3.ipynb

All done! ✨ 🍰 ✨
1 file reformatted