materialsproject / jobflow

jobflow is a library for writing computational workflows.
https://materialsproject.github.io/jobflow
Other
90 stars 25 forks source link

Increase CI sensitivity by testing both lowest-direct and highest dependency resolution strategy #640

Open janosh opened 2 weeks ago

janosh commented 2 weeks ago

related PRs: https://github.com/materialsproject/pymatgen/pull/3852 + https://github.com/CederGroupHub/chgnet/pull/159

the benefit of this kind of CI setup is that it ensures the full range of allowed version ranges in pyproject.toml actually work. e.g. resolution strategy highest could have failed since the latest versions of jobflow dependencies weren't tested before

codecov[bot] commented 2 weeks ago

Codecov Report

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

Project coverage is 91.09%. Comparing base (06a2b36) to head (e509d06).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #640 +/- ## ========================================== - Coverage 99.23% 91.09% -8.14% ========================================== Files 21 21 Lines 1573 1573 Branches 427 427 ========================================== - Hits 1561 1433 -128 - Misses 10 136 +126 - Partials 2 4 +2 ``` | [Files](https://app.codecov.io/gh/materialsproject/jobflow/pull/640?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=materialsproject) | Coverage Δ | | |---|---|---| | [src/jobflow/core/maker.py](https://app.codecov.io/gh/materialsproject/jobflow/pull/640?src=pr&el=tree&filepath=src%2Fjobflow%2Fcore%2Fmaker.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=materialsproject#diff-c3JjL2pvYmZsb3cvY29yZS9tYWtlci5weQ==) | `100.00% <100.00%> (ø)` | | ... and [4 files with indirect coverage changes](https://app.codecov.io/gh/materialsproject/jobflow/pull/640/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=materialsproject)
janosh commented 2 weeks ago

the py 3.12 CI run is failing due to a pyzmq build error

error: Failed to prepare distributions
  Caused by: Failed to fetch wheel: pyzmq==24.0.1
  Caused by: Failed to build: `pyzmq==24.0.1`
  Caused by: Build backend failed to build wheel through `build_wheel()` with exit status: 1

based on

$ pip show pyzmq              
Name: pyzmq
Version: 26.0.3
Location: /Users/janosh/.venv/py312/lib/python3.12/site-packages
Requires:
Required-by: ipykernel, jupyter-client, jupyter-server, maggma, notebook

looks like jobflow depends on pyzmq via maggma. tried to increase min maggma version to 0.69 which didn't help. @tschaume @tsmathis could we increase the min pyzmq version here to 25.1.1 which added 3.12 support

utf commented 1 week ago

Hi @janosh, thanks for this. I'm a bit confused about what's happening here. If you use resolution strategy="highest" and the strict optional dependencies at the same time, won't this just use the pinned versions in strict? I think testing the pinned versions in strict is useful btw, but happy to add additional dependency tests on top of this.

tschaume commented 1 week ago

@janosh I can't tell from your error message what exactly the build error is but if pyzmq==25.1.1 still supports python 3.9, it should be ok to increase the minimum version in maggma. Do the jupyter packages that depend on pyzmq enforce a specific minimum version on pyzmq?

janosh commented 3 days ago

If you use resolution strategy="highest" and the strict optional dependencies at the same time, won't this just use the pinned versions in strict?

@utf that's right and hence be equivalent to CI as it runs prior to this PR. the 3-fold matrix strategy does the following

- { python: "3.9", resolution: highest, extras: "tests,strict" }  # this is equivalent to the previous CI run before this PR
- { python: "3.10", resolution: lowest-direct, extras: "tests" }  # test oldest allowed versions
- { python: "3.11", resolution: highest, extras: tests }  # test latest released versions

though i just noticed a typo in the middle case (which incorrectly used strict)

@tschaume doesn't look like it, definitely not not ipykernel, nor notebook. should i submit a PR or will you bump pyzmq in maggma?

janosh commented 3 days ago

@utf looks like the new CI is working. it found jobflow is incompatible with the lowest pydantic>=2.0.1 it listed, needs at least 2.4 😄

janosh commented 3 days ago

only failing CI job now is code coverage which supposedly fell by 8%. makes no sense since no new code was added in this PR...

tschaume commented 15 hours ago

should i submit a PR or will you bump pyzmq in maggma?

@janosh feel free to go ahead and submit a PR with maggma. @Andrew-S-Rosen and @rkingsbury might have some feedback and would be able to merge your PR.