Skip to content

Conversation

@glatterf42
Copy link
Member

As announced in #975, this PR is a temporary workaround for that issue which aims to get our CI passing again.
I hope to have more time toward the end of the week to figure out a proper solution to the underlying issue.

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 ✅
  • [ ] Add, expand, or update documentation. Just CI.
  • [ ] Update release notes. Just CI.

@glatterf42 glatterf42 requested a review from khaeru September 2, 2025 10:04
@glatterf42 glatterf42 self-assigned this Sep 2, 2025
@glatterf42 glatterf42 added bug Doesn't work as advertised/unintended effects ci Continuous integration labels Sep 2, 2025
@codecov
Copy link

codecov bot commented Sep 2, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 25.9%. Comparing base (8f55fcf) to head (81101fa).
⚠️ Report is 1 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (8f55fcf) and HEAD (81101fa). Click for more details.

HEAD has 92 uploads less than BASE
Flag BASE (8f55fcf) HEAD (81101fa)
95 3
Additional details and impacted files
@@           Coverage Diff            @@
##            main    #976      +/-   ##
========================================
- Coverage   96.2%   25.9%   -70.4%     
========================================
  Files         60      60              
  Lines       5132    5132              
========================================
- Hits        4940    1330    -3610     
- Misses       192    3802    +3610     

see 57 files with indirect coverage changes

@glatterf42 glatterf42 force-pushed the fix/pin-ixmp4-without-locks branch from 0f85956 to 81101fa Compare September 2, 2025 10:27
@glatterf42
Copy link
Member Author

With the workaround (which would need to be limited to only the relevant Python versions still), we now see errors of this sort:

ERROR message_ix/tests/tools/test_sankey.py::test_map_for_sankey[ixmp4] - ModuleNotFoundError: No module named 'ixmp4.data.backend.test'

Which is understandable given the ixmp PRs' contents: it adjusted ixmp to the new ixmp4 versions in more ways than one, so we'd need to pin ixmp to a specific commit, too. Or we wait a few days for me to come back and provide better adjustments.

@khaeru
Copy link
Member

khaeru commented Sep 2, 2025

Prior to the upstream changes, ixmp[ixmp4] had these dependencies, including ixmp4 >= 0.10, < 0.11.

In order to get the same installed versions of ixmp4/behaviour as prior to iiasa/ixmp#593, the simplest and most direct thing the present PR can do is to enforce those exact same version bounds. So that would mean specifying "ixmp4 < 0.11".

Making any adjustments to message_ix (such as needed for ixmp4 v0.11.0 and later) should be out of scope here.

@glatterf42
Copy link
Member Author

True, but that won't resolve the current errors. These arise because we install ixmp from its main branch, which includes changes such as an updated import location of SqliteTestBackend. This won't be affected by further restrictions on the ixmp4 version.

@glatterf42
Copy link
Member Author

Superseded by #977.

@glatterf42 glatterf42 closed this Sep 8, 2025
@glatterf42 glatterf42 deleted the fix/pin-ixmp4-without-locks branch September 8, 2025 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Doesn't work as advertised/unintended effects ci Continuous integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants