Skip to content

Clone Scenario.scheme on all clone() calls#571

Merged
glatterf42 merged 2 commits into
mainfrom
fix/respect-scenario-scheme-on-clone
May 2, 2025
Merged

Clone Scenario.scheme on all clone() calls#571
glatterf42 merged 2 commits into
mainfrom
fix/respect-scenario-scheme-on-clone

Conversation

@glatterf42

@glatterf42 glatterf42 commented May 2, 2025

Copy link
Copy Markdown
Member

Some of the tests on iiasa/message_ix#894 fail because the .scheme property of a Scenario is not cloned when calling clone() on an IXMP4Backend (and instead set to "IXMP4-MESSAGE"), which message_ix doesn't expect. It expects "MESSAGE", but we can't hardcode that either because some ixmp tests expect something else (and we generally want to avoid hardcoding things, of course).

So this PR adjusts the clone() call on IXMP4Backend to also clone the .scheme.

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 a small internal fix.
  • [ ] Update release notes. Just a small internal fix.

@glatterf42 glatterf42 added bug backend.ixmp4 Interaction with ixmp4 via IXMP4Backend labels May 2, 2025
@glatterf42 glatterf42 added this to the 3.11 milestone May 2, 2025
@glatterf42 glatterf42 requested a review from khaeru May 2, 2025 13:02
@glatterf42 glatterf42 self-assigned this May 2, 2025
@glatterf42 glatterf42 force-pushed the fix/respect-scenario-scheme-on-clone branch from d95ec12 to d69b100 Compare May 2, 2025 13:03
@codecov

codecov Bot commented May 2, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.4%. Comparing base (c07d3f3) to head (3f39373).
⚠️ Report is 81 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##            main    #571     +/-   ##
=======================================
- Coverage   98.4%   98.4%   -0.1%     
=======================================
  Files         50      50             
  Lines       5935    5941      +6     
=======================================
+ Hits        5841    5846      +5     
- Misses        94      95      +1     
Files with missing lines Coverage Δ
ixmp/backend/ixmp4.py 90.2% <ø> (ø)
ixmp/tests/core/test_scenario.py 100.0% <100.0%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@khaeru khaeru left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this is the kind of small nuance of ixmp behaviour that previously lacked tests. If it had been tested, we would have caught this in #552. So, added the test that was missing.

Otherwise good to go once CI passes.

@glatterf42 glatterf42 merged commit 0fdd2c1 into main May 2, 2025
@glatterf42 glatterf42 deleted the fix/respect-scenario-scheme-on-clone branch May 2, 2025 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend.ixmp4 Interaction with ixmp4 via IXMP4Backend bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants