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

Fix _unicodefun patch code for Click 8.1.0 #2966

Merged
merged 3 commits into from Mar 28, 2022
Merged

Fix _unicodefun patch code for Click 8.1.0 #2966

merged 3 commits into from Mar 28, 2022

Conversation

JelleZijlstra
Copy link
Collaborator

@JelleZijlstra JelleZijlstra commented Mar 28, 2022

Fixes #2964

@ichard26 ichard26 changed the title Fix for Click 8.1.0 Fix _unicodefun patch code for Click 8.1.0 Mar 28, 2022
@github-actions
Copy link

@github-actions github-actions bot commented Mar 28, 2022

diff-shades reports zero changes comparing this PR (2ebdcc3) to main (ac7402c).


What is this? | Workflow run | diff-shades documentation

@JelleZijlstra
Copy link
Collaborator Author

@JelleZijlstra JelleZijlstra commented Mar 28, 2022

Merging now because I'm pretty sure this isn't going to break PyPy on MacOS only.

@JelleZijlstra JelleZijlstra merged commit e9681a4 into main Mar 28, 2022
38 checks passed
@JelleZijlstra JelleZijlstra deleted the nopatch branch Mar 28, 2022
@tjeck97
Copy link

@tjeck97 tjeck97 commented Mar 28, 2022

breaks black==19.10b0

ImportError: cannot import name '_unicodefun' from 'click'

@JelleZijlstra
Copy link
Collaborator Author

@JelleZijlstra JelleZijlstra commented Mar 28, 2022

@tjeck97 black 19.10b0 is old. Please upgrade.

@tjeck97
Copy link

@tjeck97 tjeck97 commented Mar 28, 2022

@tjeck97 black 19.10b0 is old. Please upgrade.

well now i'm forced to 😅 though pinning click<8.1.0 also fixes.

@mirayyuce
Copy link

@mirayyuce mirayyuce commented Mar 29, 2022

With Version: 22.1.0 I still get the same error

@JelleZijlstra
Copy link
Collaborator Author

@JelleZijlstra JelleZijlstra commented Mar 29, 2022

This PR was released in version 22.3.0.

@@ -24,7 +24,7 @@ jobs:

- name: Install diff-shades and support dependencies
run: |
python -m pip install click packaging urllib3
python -m pip install 'click<8.1.0' packaging urllib3
Copy link
Collaborator

@cooperlees cooperlees Mar 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we pin < 8.1.0 if this fixes the bug? What am I missing - Is this for diff-shades?

Copy link
Collaborator Author

@JelleZijlstra JelleZijlstra Mar 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's another bug, pallets/click#2227

Copy link
Collaborator

@ichard26 ichard26 Mar 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Click 8.1.0 breaks mypyc (and probably mypy in general) as it both deleted internal APIs we import and also had a bad typing change (Jelle would have the link).

Copy link
Collaborator

@cooperlees cooperlees Mar 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh ok - I just read this below and was super confused as to why we pinned:

- Fix Black to work with Click 8.1.0 (#2966)

Cheers

@vhosakot
Copy link

@vhosakot vhosakot commented Mar 29, 2022

This PR was released in version 22.3.0.

can confirm this issue is fixed in black 22.3.0.

@aaronbrezel
Copy link

@aaronbrezel aaronbrezel commented Mar 29, 2022

@aaronbrezel what is the issue you're seeing exactly? Because it cannot be the exact same as reported in #2964 if you actually updated because ImportError exceptions are now caught.

I appreciate the quick response!

I run black using Github Actions.

A truncated view of my installation process:

Collecting black>=22.3.0
  Downloading black-22.3.0-cp39-cp39-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (1.5 MB)
...
Successfully installed ... black-22.3.0 ... click-8.1.0 ...

The action then runs:

pre-commit install
pre-commit run --all-files

to check for proper linting and formatting.

This results in the following error:

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

Traceback (most recent call last):
  File "/home/runner/.cache/pre-commit/repo4s5v_ps5/py_env-python3.9/bin/black", line 8, in <module>
    sys.exit(patched_main())
  File "/home/runner/.cache/pre-commit/repo4s5v_ps5/py_env-python3.9/lib/python3.9/site-packages/black/__init__.py", line 1282, in patched_main
    patch_click()
  File "/home/runner/.cache/pre-commit/repo4s5v_ps5/py_env-python3.9/lib/python3.9/site-packages/black/__init__.py", line 1268, in patch_click
    from click import _unicodefun  # type: ignore
ImportError: cannot import name '_unicodefun' from 'click' (/home/runner/.cache/pre-commit/repo4s5v_ps5/py_env-python3.9/lib/python3.9/site-packages/click/__init__.py)

@jack1142
Copy link
Contributor

@jack1142 jack1142 commented Mar 29, 2022

@aaronbrezel pre-commit uses an isolated virtual environment so you need to make sure that you update the version of Black in .pre-commit-config.yaml.

@aaronbrezel
Copy link

@aaronbrezel aaronbrezel commented Mar 29, 2022

@aaronbrezel pre-commit uses an isolated virtual environment so you need to make sure that you update the version of Black in .pre-commit-config.yaml.

Ah that's it! Thank you very much for the quick help

aucampia added a commit to aucampia/rdflib that referenced this issue Mar 29, 2022
aucampia added a commit to aucampia/rdflib that referenced this issue Mar 29, 2022
See this psf/black#2966

Also simplified pre-commit config of black based on a suggestion from
https://github.com/jack1142 in psf/black#2493 (comment)
aucampia added a commit to aucampia/rdflib that referenced this issue Mar 29, 2022
See this psf/black#2966

Also simplified pre-commit config of black based on a suggestion from
https://github.com/jack1142 in psf/black#2493 (comment)
ashleysommer pushed a commit to RDFLib/rdflib that referenced this issue Mar 30, 2022
See this psf/black#2966

Also simplified pre-commit config of black based on a suggestion from
https://github.com/jack1142 in psf/black#2493 (comment)
intgr added a commit to intgr/django-stubs that referenced this issue Mar 30, 2022
Closes typeddjango#896.

Black 22.3.0 fixes incompatibility with Click 8.1.0:
psf/black#2966

This currently causes the CI to fail, e.g. https://github.com/typeddjango/django-stubs/runs/5756075543
puddly added a commit to puddly/zigpy that referenced this issue Mar 30, 2022
sobolevn pushed a commit to typeddjango/django-stubs that referenced this issue Mar 31, 2022
* Update Black dependency to fix CI lint

Closes #896.

Black 22.3.0 fixes incompatibility with Click 8.1.0:
psf/black#2966

This currently causes the CI to fail, e.g. https://github.com/typeddjango/django-stubs/runs/5756075543

* Reformat with Black v22
LukasNickel added a commit to cta-observatory/ctapipe that referenced this issue Apr 5, 2022
- Click 8.1 broke black
- Fixed in black 22.3: psf/black#2966
nmenardg-keeper added a commit to nmenardg-keeper/python-lint-annotate that referenced this issue Jun 16, 2022
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 this pull request may close these issues.

9 participants