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

CI: Use uv for installing deps #752

Merged
merged 2 commits into from
Jun 30, 2024
Merged

Conversation

danielhollas
Copy link
Contributor

Use uv installer to speed up installing dependencies for tests + some other CI tweaks brought over from AWB:

  • Use FORCE_COLOR to get colored logs in Github UI
  • Update actions
  • Don't collect code coverage from tests/ folders

- Use FORCE_COLOR to get colored logs in Github UI
- Update actions
- Don't collect code coverage from tests/ folders
Copy link

codecov bot commented Jun 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.28%. Comparing base (f63da7d) to head (ad77e3c).
Report is 138 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (f63da7d) and HEAD (ad77e3c). Click for more details.

HEAD has 2 uploads less than BASE
Flag BASE (f63da7d) HEAD (ad77e3c)
python-3.8 2 0
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #752       +/-   ##
===========================================
- Coverage   80.73%   68.28%   -12.45%     
===========================================
  Files          49       45        -4     
  Lines        3415     4143      +728     
===========================================
+ Hits         2757     2829       +72     
- Misses        658     1314      +656     
Flag Coverage Δ
python-3.10 68.28% <ø> (-12.45%) ⬇️
python-3.8 ?
python-3.9 68.31% <ø> (-12.46%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@superstar54 superstar54 left a comment

Choose a reason for hiding this comment

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

Thanks @danielhollas for the work. I don't understand some of the changes, could you address my comments?


- name: Run pytest
run: pytest -v tests --cov
run: pytest -v tests --cov=aiidalab_qe
Copy link
Member

Choose a reason for hiding this comment

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

Why add aiidalab_qe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Citing from pytest --help

  --cov=[SOURCE]   Path or package name to measure during execution.                                                 
                                 Use --cov= to not do any source filtering and record everything.

Without specifying the package there are two problems:

  1. (minor) Coverage of tests/ folder is collected, which skews the overall coverage percentage.
  2. If you don't specify the package, some files from the aiidalab_qe package might be omitted from the statistics.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that aiida-core uses the same approach.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's how things look in Codecov without this change:

image

https://app.codecov.io/gh/aiidalab/aiidalab-qe

Comment on lines +13 to +19
commands:
- asdf plugin add uv
- asdf install uv 0.2.13
- asdf global uv 0.2.13
- uv venv
- uv pip install -r docs/requirements.txt
- .venv/bin/python -m sphinx -W --keep-going -d _build/doctrees -D language=en -b html docs/source $READTHEDOCS_OUTPUT/html
Copy link
Member

Choose a reason for hiding this comment

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

Are there big benefits to this change? Otherwise it's better to use the one from the official readthedocs docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It speeds up the build from ~30s to 5s. The configuration suggested here actually comes from the official docs https://docs.readthedocs.io/en/stable/build-customization.html#install-dependencies-with-uv

But it would be fine to revert this change if you don't like it.

@danielhollas danielhollas requested a review from superstar54 June 30, 2024 08:25
Copy link
Member

@superstar54 superstar54 left a comment

Choose a reason for hiding this comment

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

Thanks @danielhollas for the detailed explanation! Nice work!

@danielhollas danielhollas merged commit 39d411b into aiidalab:main Jun 30, 2024
14 of 15 checks passed
@danielhollas danielhollas deleted the ci-tweaks branch June 30, 2024 19:00
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