lbl-anp / becquerel

Becquerel is a Python package for analyzing nuclear spectroscopic measurements.
Other
43 stars 16 forks source link

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

Closed markbandstra closed 2 years ago

markbandstra commented 2 years ago

Fixes #324

markbandstra commented 2 years ago

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 commented 2 years ago

I don't see the error on my machine:

jccurtis commented 2 years ago

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

markbandstra commented 2 years ago

Good point, here is my machine:

jvavrek commented 2 years ago

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

jccurtis commented 2 years ago

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 commented 2 years ago

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

jccurtis commented 2 years ago

@markbandstra I think so

jccurtis commented 2 years ago

This also occurs on python3.10 with the same steps

jvavrek commented 2 years ago

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

jccurtis commented 2 years ago

FYI This is supposedly fixed but I do not see the fix with the most updated packages: https://github.com/psf/black/pull/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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

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

markbandstra commented 2 years ago

Great find @micahfolsom !