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

Enable the latest Compendium materials JSON format to be read #328

Merged
merged 2 commits into from Apr 4, 2022

Conversation

markbandstra
Copy link
Member

Fixes #324

@markbandstra markbandstra self-assigned this Mar 28, 2022
@markbandstra
Copy link
Member Author

This PR is failing due to an issue I saw locally. I figured it was an error in pre-commit, black, or click but could not fix it and bypassed it. Here is the relevant error:

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

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

@jvavrek
Copy link
Contributor

jvavrek commented Mar 28, 2022

I don't see the error on my machine:

  • Python 3.8.10
  • pre-commit 2.12.0
  • black 22.1.0
  • click 8.0.1

@jccurtis
Copy link
Collaborator

@jvavrek yep I don't either BUT click was just updated a few hours ago: https://pypi.org/project/click/8.1.0/

@markbandstra
Copy link
Member Author

Good point, here is my machine:

  • Python 3.9.12
  • pre-commit 2.17.0
  • black 22.1.0
  • click 8.0.4

@jvavrek
Copy link
Contributor

jvavrek commented Mar 28, 2022

I just upgraded to click 8.0.4 (latest available on conda-forge) and I still don't see the error.

@jccurtis
Copy link
Collaborator

Confirmed on a fresh install:

# Inside bq repo
conda create -n bq-test python=3.9
conda activate bq-test
pip install -r requirements-dev.txt
pip install -e .
pre-commit install
pre-commit run --all

gives:

❯ pre-commit run --all
Trim Trailing Whitespace.................................................Passed
Fix End of Files.........................................................Passed
Check for added large files..............................................Passed
Check JSON...........................................(no files to check)Skipped
Check Toml...............................................................Passed
Check Yaml...............................................................Passed
Check for merge conflicts................................................Passed
Check Yaml...............................................................Passed
Mixed line ending........................................................Passed
Fix requirements.txt.....................................................Passed
black....................................................................Failed
- hook id: black
- exit code: 1

Traceback (most recent call last):
  File "<removed>/py_env-python3.9/bin/black", line 8, in <module>
    sys.exit(patched_main())
  File "<removed>/py_env-python3.9/lib/python3.9/site-packages/black/__init__.py", line 1423, in patched_main
    patch_click()
  File "<removed>/py_env-python3.9/lib/python3.9/site-packages/black/__init__.py", line 1409, in patch_click
    from click import _unicodefun
ImportError: cannot import name '_unicodefun' from 'click' (<removed>/py_env-python3.9/lib/python3.9/site-packages/click/__init__.py)

autoflake................................................................Passed
flake8...................................................................Passed
markdownlint-fix.........................................................Passed
prettier.................................................................Passed

@markbandstra
Copy link
Member Author

Maybe a Python3.9 issue? That's a commonality between my machine and the failing test runner.

@jccurtis
Copy link
Collaborator

@markbandstra I think so

@jccurtis
Copy link
Collaborator

This also occurs on python3.10 with the same steps

@jvavrek
Copy link
Contributor

jvavrek commented Mar 28, 2022

I just upgraded to click 8.0.4 (latest available on conda-forge) and I still don't see the error.

Scratch this, my pre-commit was skipping a bunch of stuff. I can replicate the error with

  • Python 3.8.10
  • black, 22.1.0 (compiled: no)
  • pre-commit 2.12.0
  • click 8.0.1

@jccurtis
Copy link
Collaborator

FYI This is supposedly fixed but I do not see the fix with the most updated packages: psf/black#2966

❯ pip freeze
algopy==0.5.7
asteval==0.9.26
attrs==21.4.0
beautifulsoup4==4.10.0
-e git+https://github.com/lbl-anp/becquerel.git@62ddabd2db84d1e67fc1ad1f80433e13f0785b39#egg=becquerel
black==22.3.0
bump2version==1.0.1
certifi==2020.6.20
cfgv==3.3.1
charset-normalizer==2.0.12
click==8.1.0
coverage==6.3.2
cycler==0.11.0
distlib==0.3.4
filelock==3.6.0
flake8==4.0.1
fonttools==4.31.2
future==0.18.2
h5py==3.6.0
html5lib==1.1
identify==2.4.12
idna==3.3
iminuit==2.11.2
iniconfig==1.1.1
kiwisolver==1.4.2
llvmlite==0.38.0
lmfit==1.0.3
lxml==4.8.0
matplotlib==3.5.1
mccabe==0.6.1
mypy-extensions==0.4.3
nodeenv==1.6.0
numba==0.55.1
numdifftools==0.9.40
numpy==1.21.5
packaging==21.3
pandas==1.4.1
pathspec==0.9.0
patsy==0.5.2
Pillow==9.0.1
platformdirs==2.5.1
pluggy==1.0.0
pre-commit==2.17.0
py==1.11.0
pycodestyle==2.8.0
pyflakes==2.4.0
pyparsing==3.0.7
pytest==6.2.5
pytest-black==0.3.12
pytest-cov==3.0.0
pytest-rerunfailures==10.2
python-dateutil==2.8.2
pytz==2022.1
PyYAML==6.0
requests==2.27.1
scipy==1.8.0
six==1.16.0
soupsieve==2.3.1
statsmodels==0.13.2
toml==0.10.2
tomli==2.0.1
uncertainties==3.1.6
urllib3==1.26.9
virtualenv==20.14.0
webencodings==0.5.1

@jvavrek
Copy link
Contributor

jvavrek commented Mar 28, 2022

Comments in that psf/black link suggest 8.0.1 should work, so it's strange that I'm still seeing it with click==8.0.1

@jccurtis
Copy link
Collaborator

@jvavrek yep I see this with click==8.0.1 and click==8.0.4. For reference:

black==22.3.0
pre-commit==2.17.0

(same as the pip freeze from above)

@micahfolsom
Copy link
Collaborator

@jccurtis Not sure what's going on there - bumping black to 22.3 fixed the issue for me, both locally, and in the pre-commit config.

@jccurtis
Copy link
Collaborator

@micahfolsom beat me to it ... I was writing the PR 🤣: #329

The issue in the testing above was a bit of a 🤦 ... we weren't updating the pre-commit config

@jccurtis
Copy link
Collaborator

@micahfolsom let's get #329 in first. It makes the changelog easier to read 😄

@markbandstra
Copy link
Member Author

Great find @micahfolsom !

Copy link
Collaborator

@jccurtis jccurtis left a comment

Choose a reason for hiding this comment

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

Nice work here. Thanks for the new test

@markbandstra markbandstra merged commit 2b728ff into main Apr 4, 2022
@markbandstra markbandstra deleted the fix-compendium-json-errors branch April 4, 2022 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Latest Compendium materials JSON causes errors
4 participants