Save minutes by avoiding duplicate test runs

[glatterf42]

This PR removes the trigger for CI runs on pushed to main to avoid duplicate test runs. This avoids duplicate runs.

In order for this to work properly, we should make sure that

• main can only be reached via PRs
• PRs need to be up to date with main
• PRs require several tests to be successful before merging

We might also want to

• expand the number of tests we require to be successful

All of which can be done via the settings -> branches.

For now, we require the py3.11 checks for macos, ubuntu, and windows and message_ix/main to be successful. We should include checks for message_ix v3.8.0 (or only use main) and could try using py3.12, too.

How to review

• Read the diff and note that the CI checks all pass.

PR checklist

• Continuous integration checks all ✅
• [ ] Add or expand tests; coverage checks both ✅ Just changing how often tests are run.
• [ ] Add, expand, or update documentation. Just changing how often tests are run.
• [ ] Update release notes. Just changing how often tests are run.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.4%. Comparing base (583d3e9) to head (8535525).

Additional details and impacted files

@@          Coverage Diff          @@
##            main    #150   +/-   ##
=====================================
  Coverage   75.4%   75.4%           
=====================================
  Files         93      93           
  Lines       6211    6214    +3     
=====================================
+ Hits        4684    4687    +3     
  Misses      1527    1527           

Files	Coverage Δ
message_ix_models/tests/test_util.py	100.0% <100.0%> (ø)

... and 1 file with indirect coverage changes

[khaeru]

We should include checks for message_ix v3.8.0 (or only use main)

Yes, including both the latest release and main should be our general rule.

and could try using py3.12, too.

Worth a try! I not yet sure if all of the direct dependencies here are compatible with Python 3.12, but we could find out.

[glatterf42]

Let's see how the 3.12 tests are doing. We might have to re-trigger a check evaluation to get rid of the '3.11-required' checks and update the branch protection rule if we want to keep the 3.12 version.

[khaeru]

We might have to re-trigger a check evaluation to get rid of the '3.11-required' checks and update the branch protection rule

I find if I modify the required checks, save settings, and then refresh the PR page, then the new requirements are shown without the need to re-run any jobs.

However it looks like there are several failures, so one option would be to park some notes in an issue to be addressed separately and limit this PR to adding ixmp/message_ix 3.8.0. Or try to also address them in this PR—up to you.

[glatterf42]

This PR now refines the pytest workflow usage of python-version. A lot of the failures originated from the incompatibility of (ixmp, message_ix) < 3.8.0 with python 3.12 and we are not requiring this compatibility anywhere, so this workflow shouldn't require it either.

[glatterf42]

@khaeru I'm not sure how to handle the Code Quality issue (which is caused by pint missing library stubs or py.typed markers). Locally, this is the same for me and since pint doesn't support 3.12 officially yet, there might not be much we can do aside from adding it to tool.mypy.overrides in pyproject.toml until official support is added.
What confuses me, though, is that recent runs of code quality checks on ixmp seem to have installed python 3.12 and successfully run mypy, even though pint is a dependency there, too, and not excluded via pyproject.toml.

[khaeru]

• Pint does include py.typed, for about 3 years: hgrecco/pint@c60e1af
• The approach for genno is to include it in the list of packages in pre-commit: https://github.com/khaeru/genno/blob/main/.pre-commit-config.yaml#L10 —there is no exclusion in pyproject.toml.
• In the .pre-commit-config.yaml additional dependencies for mypy, there are two options:
  • If a package like lxml-stubs or types-requests exists, then use this. This does not install e.g. lxml or requests or any of their dependencies.
  • Otherwise, use packages directly, like xarray. This results in the full, actual package being installed in the pre-commit venv used for mypy—thus also all of its dependencies.
• The ixmp .pre-commit-config.yaml includes genno, which has a mandatory dependency on pint, so pint is installed in the pre-commit venv used for mypy.
• Here, the .pre-commit-config.yaml includes ixmp, but there is no mandatory (direct or indirect) dependency on pint, so pint is not installed. Two options would be:
  • Add pint directly to .pre-commit-config.yaml (preferred)
  • Change the ixmp line to something like ixmp[report] @ git+https://github.com/iiasa/ixmp.git@main. IMHO indirect dependencies are best for pyproject.toml/PyPI, but direct ones are fine for mypy.

[glatterf42]

Thanks for the explanation. If I didn't misunderstand it entirely, f7d54e3 should be exactly the change you suggested. However, this doesn't solve the problem here (and probably won't for message_data, either, since the error message (also for message_data)) reads:

message_data/testing.py:7: error: Skipping analyzing "pint": module is installed, but missing library stubs or py.typed marker  [import-untyped]

[glatterf42]

As far as I can tell, there's now no difference between the failing Code quality run here and e.g. this successful one for ixmp except for the packages being passed to pre-commit's mypy-venv.

[INFO] Initializing environment for local:mypy >= 1.8.0,plotnine,pytest,sdmx1,types-PyYAML,types-tqdm,ixmp @ git+https://github.com/iiasa/ixmp.git@main,message-ix @ git+https://github.com/iiasa/message_ix.git@main.

here vs

[INFO] Initializing environment for local:mypy >= 1.8.0,genno,GitPython,nbclient,pytest,sphinx,xarray.

there. Since it doesn't seem to be missing pint, my guess is that one of the dependencies here (most likely message-ix) depends on pint in a way that just ixmp doesn't require to work.

P.S.: there's also an environment variable called depth set to 100 for ixmp, but that doesn't seem to affect this workflow.

[glatterf42]

Okay, so for the last CI check before the force-push, we had:

message-ix-models/.github/workflows/pytest.yaml

Line 111 in cba07b2

if: github.event_name == 'schedule' # Comment this line to run on a PR

commented out and

message-ix-models/.pre-commit-config.yaml

Lines 11 to 12 in cba07b2

	entry: bash -c ". ${PRE_COMMIT_MYPY_VENV:-/dev/null}/bin/activate 2>/dev/null; mypy $0 $@"
	additional_dependencies:

appended with ; python -m pip list at the end of the quotation marks. For some reason, this made the code quality check successful, but it's failing again now.

[glatterf42]

There it is again: adding ; python -m pip list to the pre-commit-config bash mypy call makes the code quality CI check successful. Let's see if the same happens for message_data.

[glatterf42]

@khaeru Gotta be honest: I don't understand what's going on here, but the tests are successful here and over at https://github.com/iiasa/message_data/pull/533, so it seems like we can adapt the required-successful checks to check python 3.12 and officially support 3.12 throughout the message stack :)

[glatterf42]

The publish workflow doesn't run because https://github.com/iiasa/message-ix-models/actions/runs/8433520273 is neither starting (despite the job it's waiting for being finished) nor able to get cancelled. If it's okay with you, @khaeru, I'd change the required checks and merge this, anyway.

[khaeru]

Could you please adjust the required statuses in the repo settings? These have different names now with these changes to the workflow. Otherwise good to go.