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.

Sign up for GitHub

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

Jump to bottom

Bump Python packages version requirements #4115

Merged
merged 4 commits into from
Feb 19, 2021
Merged

Bump Python packages version requirements #4115

merged 4 commits into from
Feb 19, 2021

Conversation

jngrad
Copy link
Member

@jngrad jngrad commented Feb 12, 2021

Description of changes:

@jngrad
python: Remove numpy aliases of builtin types
92c14d9 
They are deprecated and throw warnings in numpy v1.20. See the
complete list in the release notes:
https://numpy.org/devdocs/release/1.20.0-notes.html#using-the-aliases-of-builtin-types-like-np-int-is-deprecated
@jngrad
python: Use package versions from Ubuntu 20.04
b0e7a91
@jngrad
docs: Add guard against unsupported pint versions
390beaf 
Old pint versions are known to be incompatible with numpy.
See the pint release notes for the complete list of numpy
issues: https://github.com/hgrecco/pint/blob/master/CHANGES

For example in pint 0.8.1, np.cbrt() silently converts a
pint.Quantity object to a float object, while np.sqrt()
doesn't. Also, several quantities like ureg.molar and
ureg.avogadro_constant were only introduced in pint 0.9.
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

@jngrad
Copy link
Member Author

jngrad commented Feb 12, 2021

pylint is taking 4 to 12 min to run in the CI environment, while outside the CI environment (using the same docker image on the same runner), it takes 15 seconds. When running a debug session on the runner, executing pylint on a single file takes 3 min:

$> time pylint --score=no --reports=no --output-format=text testsuite/python/save_checkpoint.py
real    3m12.496s
user    0m6.168s
sys     0m0.217s

Re-running the CI job multiple times decreases the pylint runtime, which is now down to 26 seconds for save_checkpoint.py in the debug session and around 2 min for pre-commit in the CI job. Is pre-commit using a persistent cache?

@jngrad jngrad added the Python label Feb 12, 2021
@jngrad jngrad added this to the Espresso 4.2 milestone Feb 12, 2021
@KaiSzuttor
Copy link
Member

As far as i understand our CI configuration, we are not using any persistent cache. Thus, I cannot explain your observations.

@jngrad
Copy link
Member Author

jngrad commented Feb 16, 2021

The docker images run with the following bind: /scratch/cache:/cache. However the folder ~/.cache/pre-commit is created the first time we call pre-commit, so it's not an issue with caching. The issue probably comes from Python, it takes too long to load the pylint tool. In CI:

espresso@coyote:~$ time pylint --version
pylint 2.4.4
astroid 2.3.3
Python 3.8.5 (default, Jul 28 2020, 12:59:40) 
[GCC 9.3.0]

real    0m20.020s
user    0m0.742s
sys     0m0.091s

Outside CI:

espresso@8e4a7ed3e5bd:~$ time pylint --version
pylint 2.4.4
astroid 2.3.3
Python 3.8.5 (default, Jul 28 2020, 12:59:40) 
[GCC 9.3.0]

real	0m0.458s
user	0m0.401s
sys	0m0.056s

@jngrad
Copy link
Member Author

jngrad commented Feb 19, 2021

@KaiSzuttor can this be merged?

@KaiSzuttor KaiSzuttor added the automerge Merge with kodiak label Feb 19, 2021
@kodiakhq kodiakhq bot merged commit b4e94a3 into espressomd:python Feb 19, 2021
@jngrad jngrad deleted the update-python-pkgs-ubu20 branch January 18, 2022 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KaiSzuttor KaiSzuttor KaiSzuttor approved these changes

Assignees
No one assigned
Labels
automerge Merge with kodiak Python
Projects
None yet
Milestone
Espresso 4.2.0
Development

Successfully merging this pull request may close these issues.
2 participants
@jngrad @KaiSzuttor