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 CI and correct the setup.json #110

Merged
merged 1 commit into from
Apr 2, 2021
Merged

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Apr 1, 2021

Various fixes for the CI workflow

  • Update version of actions checkout and setup-python to v2
  • install Postgres and RabbitMQ through services. Note that we still
    need to install Postgres manually as well for pgtest to work.
  • Remove manual install of numpy which was a temporary workaround
    that is now obsolete.
  • Address the warnings from pylint
  • Fix the pre-commit job: multiline run statement did not use |
  • Add Python 3.9 to the tests matrix

Also the setup.json was

  • Remove compatibility for Python 3.5 from classifiers as it is EOL.
    The tests were also not being run for this version, as the dependency
    aiida-testing does not support it.
  • Add compatibility for Python 3.9 to classifiers
  • Add the python_requires>=3.6 key
  • Install aiida-core with the atomic_tools extra. This allows to
    remove the explicit installs of ase, pymatgen and PyCifRW.
  • Add temporary upper limit on sqlalchemy which is a workaround for
    incompatibility with sqlalchemy-utils.
  • Add upper limit on masci-tools because of breaking changes.
  • Remove upper limit of numpy which is not necessary and causes
    conflicts with other packages for aiida-common-workflows.

Various fixes for the CI workflow

 * Update version of actions `checkout` and `setup-python` to v2
 * install Postgres and RabbitMQ through services. Note that we still
   need to install Postgres manually as well for `pgtest` to work.
 * Remove manual install of `numpy` which was a temporary workaround
   that is now obsolete.
 * Address the warnings from pylint
 * Fix the `pre-commit` job: multiline run statement did not use `|`
 * Add Python 3.9 to the tests matrix

Also the `setup.json` was

 * Remove compatibility for Python 3.5 from classifiers as it is EOL.
   The tests were also not being run for this version, as the dependency
   `aiida-testing` does not support it.
 * Add compatibility for Python 3.9 to classifiers
 * Add the `python_requires>=3.6` key
 * Install `aiida-core` with the `atomic_tools` extra. This allows to
   remove the explicit installs of `ase`, `pymatgen` and `PyCifRW`.
 * Add temporary upper limit on `sqlalchemy` which is a workaround for
   incompatibility with `sqlalchemy-utils`.
 * Add upper limit on `masci-tools` because of breaking changes.
 * Remove upper limit of `numpy` which is not necessary and causes
   conflicts with other packages for `aiida-common-workflows`.
@sphuber
Copy link
Contributor Author

sphuber commented Apr 1, 2021

This PR supersedes #109 because the CI was broken and I needed to fix quite a few additional problems to get all tests to pass.

@broeder-j and @Tseplyaev I think these changes should fix the issue that we have in installing aiida-fleur with aiida-common-workflows in the Quantum Mobile. Do you think it would be ok to merge these changes and make a release? Or did I misunderstand your reasoning for putting an upper limit on numpy? I see that Vasily set the upper limit in this commit d3a64d0 but the message didn't specify why.

@codecov
Copy link

codecov bot commented Apr 1, 2021

Codecov Report

Merging #110 (9eed552) into master (df33e9a) will increase coverage by 0.00%.
The diff coverage is 22.22%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #110   +/-   ##
=======================================
  Coverage   68.56%   68.57%           
=======================================
  Files          71       71           
  Lines       10501    10501           
=======================================
+ Hits         7200     7201    +1     
+ Misses       3301     3300    -1     
Impacted Files Coverage Δ
aiida_fleur/workflows/create_magnetic_film.py 25.00% <0.00%> (ø)
aiida_fleur/workflows/dmi.py 42.33% <0.00%> (ø)
aiida_fleur/workflows/eos.py 64.70% <0.00%> (ø)
aiida_fleur/workflows/mae.py 25.00% <0.00%> (ø)
aiida_fleur/workflows/mae_conv.py 25.80% <0.00%> (ø)
aiida_fleur/workflows/ssdisp.py 44.44% <0.00%> (ø)
aiida_fleur/workflows/ssdisp_conv.py 23.62% <0.00%> (ø)
aiida_fleur/tools/common_fleur_wf.py 83.48% <100.00%> (+0.31%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update df33e9a...9eed552. Read the comment docs.

@broeder-j
Copy link
Member

Hi @sphuber. Thanks for doing this. I can only suspect that the upper limit on numpy at that time was due to dependency clashes with numpy, ase, pymatgen and bokeh, but I do not know. Since these are no major changes and so far there is no reason for the upper limit now anymore that I see and this is holding back the common-workflows, I am fine with a quick merge and release ASP after checking back with @Tseplyaev .

@broeder-j broeder-j merged commit a548981 into JuDFTteam:master Apr 2, 2021
@sphuber sphuber deleted the fix/ci branch April 7, 2021 17:09
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.

2 participants