Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
chamilad opened this issue Apr 4, 2022 · 8 comments
Closed

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

chamilad opened this issue Apr 4, 2022 · 8 comments

Comments

@chamilad
Copy link

chamilad commented Apr 4, 2022

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
Copy link
Author

chamilad commented Apr 4, 2022

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
Copy link
Collaborator

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
Copy link
Author

chamilad commented Apr 5, 2022

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
Copy link
Collaborator

Can you show your precommit config file please?

@s-weigand
Copy link
Contributor

s-weigand commented Apr 5, 2022

@chamilad black fixed this issue by now.
The hook from nbqa does not pin the back version

- id: nbqa-black
name: nbqa-black
description: "Run 'black' on a Jupyter Notebook"
entry: nbqa black
language: python
require_serial: true
types: [jupyter]
additional_dependencies: [black]

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?

@MarcoGorelli MarcoGorelli pinned this issue Apr 6, 2022
@chamilad
Copy link
Author

chamilad commented Apr 6, 2022

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
Copy link
Collaborator

No worries, thanks for the report!

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

@dnoliver
Copy link

dnoliver commented Aug 19, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants