matthewfeickert commented 4 months ago

Description

Use uv to try to speed up the installs of all the Python dependencies.

Fall back to using pip for Python 3.8 until https://github.com/astral-sh/uv/issues/2062 is resolved.

Apply subtle changes to install commands in .github/workflows/dependencies-head.yml.

uv pip install --upgrade will try to upgrade all dependencies of the target package as well, which for the dependencies-head workflow isn't the goal. So remove the --upgrade from calls that also install from the scientific-python-nightly-wheels package index when testing only particular packages.
up pip and pip have different behavior with regards to --extra-index-url, as uv pip gives --extra-index-url priority over --index-url, where pip does not give priority to either. Use this with uv pip to give priority to the scientific-python-nightly-wheels package index.
- c.f. https://github.com/scientific-python/upload-nightly-action/issues/76

Checklist Before Requesting Reviewer

[x] Tests are passing
[x] "WIP" removed from the title of the pull request
[x] Selected an Assignee for the PR to be responsible for the log summary

Before Merging

For the PR Assignees:

[x] Summarize commit messages into a comprehensive review of the PR

* Use 'uv pip' for all calls to 'pip install' and 'pip uninstall' in
  CI workflows.
   - c.f. https://github.com/astral-sh/uv/
   - Still use pip for Python 3.8 until https://github.com/astral-sh/uv/issues/2062
     is resolved.
* Apply subtle changes to install commands in .github/workflows/dependencies-head.yml.
   - 'uv pip install --upgrade' will try to upgrade all dependencies of the target
     package as well, which for the dependencies-head workflow isn't the goal. So
     remove the '--upgrade' from calls that also install from the
     scientific-python-nightly-wheels package index when testing only particular packages.
   - 'up pip' and 'pip' have different behavior with regards to --extra-index-url, as
     'uv pip' gives --extra-index-url priority over --index-url, where 'pip' does
     not give priority to either. Use this with 'uv pip' to give priority to the
     scientific-python-nightly-wheels package index.
* Add uv to the 'develop' extras.

matthewfeickert commented 4 months ago

Need https://github.com/astral-sh/uv/issues/1526 or an alternative for this to work first.

Edit: Can do this for the time being:

    - name: Set up Python
      uses: actions/setup-python@v5
      with:
        python-version: ${{ matrix.python-version }}

    - name: Set the VIRTUAL_ENV variable for uv to work
      run: |
        echo "VIRTUAL_ENV=${Python_ROOT_DIR}" >> $GITHUB_ENV

    - name: Install dependencies
      run: |
        python -m pip install uv
        python -m uv pip install --upgrade <stuff>

which comes from https://github.com/astral-sh/uv/issues/1386#issuecomment-1947801083.

matthewfeickert commented 4 months ago

Also needs https://github.com/astral-sh/uv/pull/1531 so will have to wait for the next release of uv (if that PR goes in).

matthewfeickert commented 4 months ago

Given https://github.com/astral-sh/uv/pull/1531#issuecomment-1950752750 we also need https://github.com/astral-sh/uv/issues/313 to go in.

matthewfeickert commented 4 months ago

Okay with https://github.com/astral-sh/uv/issues/1526 in and using the

install "pyhf[extras] @ ."

pattern over

install ".[extras]"

pattern I think this should be good.

edit: No, uv pip is resolving dependencies for tensorflow different than pip does and this breaks.

matthewfeickert commented 4 months ago

Reported failure in https://github.com/astral-sh/uv/issues/2062

matthewfeickert commented 3 months ago

Also blocked by https://github.com/astral-sh/uv/issues/2685

matthewfeickert commented 2 months ago

So this is really blocked by papermill given https://github.com/nteract/papermill/pull/789 is in and so just waiting for a papermill v2.5.1 or v2.6.0 release.

matthewfeickert commented 2 months ago

With the release of papermill v2.6.0 (https://github.com/nteract/papermill/issues/785#issuecomment-2080971385) and relaxing to

...
    "papermill>=2.5.0",
    "scrapbook>=0.5.0",
...

https://github.com/astral-sh/uv/issues/2685 is sidestepped, but the issue with TensorFlow dependency resolution in https://github.com/astral-sh/uv/issues/2062 is still blocking.

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 98.21%. Comparing base (682ce76) to head (854906d).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #2444 +/- ## ======================================= Coverage 98.21% 98.21% ======================================= Files 69 69 Lines 4543 4543 Branches 804 804 ======================================= Hits 4462 4462 Misses 48 48 Partials 33 33 ``` | [Flag](https://app.codecov.io/gh/scikit-hep/pyhf/pull/2444/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep) | Coverage Δ | | |---|---|---| | [contrib](https://app.codecov.io/gh/scikit-hep/pyhf/pull/2444/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep) | `97.79% <ø> (ø)` | | | [doctest](https://app.codecov.io/gh/scikit-hep/pyhf/pull/2444/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep) | `98.08% <ø> (ø)` | | | [unittests-3.10](https://app.codecov.io/gh/scikit-hep/pyhf/pull/2444/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep) | `96.23% <ø> (ø)` | | | [unittests-3.11](https://app.codecov.io/gh/scikit-hep/pyhf/pull/2444/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep) | `96.23% <ø> (ø)` | | | [unittests-3.12](https://app.codecov.io/gh/scikit-hep/pyhf/pull/2444/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep) | `96.23% <ø> (ø)` | | | [unittests-3.8](https://app.codecov.io/gh/scikit-hep/pyhf/pull/2444/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep) | `96.25% <ø> (ø)` | | | [unittests-3.9](https://app.codecov.io/gh/scikit-hep/pyhf/pull/2444/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep) | `96.27% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

matthewfeickert commented 2 months ago

Okay, I think this is finally working as expected across all workflows now.

matthewfeickert commented 2 months ago

Some examples of install time drops:

OS	Python	`pip`	`uv pip`
Ubuntu	3.12	2m 57 s	38 s
macOS (Intel)	3.12	2m 51 s	50 s
macOS (Apple silicon)	3.12	1m 6 s	16 s

:rocket:

matthewfeickert commented 2 months ago

I'm going to approve and merge this myself. As always, PRs approved by a single core dev can be reverted as needed by the rest of the dev team.

scikit-hep / pyhf

ci: Use uv for all pip installs #2444

Description

Checklist Before Requesting Reviewer

Before Merging

Codecov Report