-
Notifications
You must be signed in to change notification settings - Fork 166
Test against ixmp4 #894
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Test against ixmp4 #894
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #894 +/- ##
=======================================
- Coverage 95.9% 90.6% -5.4%
=======================================
Files 48 53 +5
Lines 4487 4988 +501
=======================================
+ Hits 4305 4521 +216
- Misses 182 467 +285
|
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as resolved.
This comment was marked as resolved.
|
At iiasa/ixmp#557, @glatterf42 asked:
To expand on iiasa/ixmp#557 (comment): I briefly played around with #873 last year. Via that PR or something else, there are multiple possible solutions. I'm not sure right now which achieves our goal of minimizing breaking changes:
While (1) is not 100% 'clean' in the sense that message_ix.Scenario shouldn't need to know or care what backend it is talking to, the limitations of the Java code (see the other PR) might make (2) unworkable, in which case (1) would be, I think, the simplest compromise option. |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
c54e5bc to
6e6060b
Compare
6f866a2 to
466a632
Compare
26c9360 to
8461383
Compare
- Use variable in install steps. - Expand documentation comment. - Temporarily add iiasa/ixmp4#171 branch for #894.
Simplify repeated conditionals in .core.Scenario.
- Move IXMP4-specific initialization steps from Scenario.__init__ to follow generic initialization steps already in MESSAGE.initialize(). - Move IXMP4-specific pre-model-run steps from Scenario.solve() to an implementation of Model.run(), provided for this purpose. - Remove unused add_or_extend_item_list(), tests.
- Replace "NOTE this assumes an IXMP4Backend" with call to on_ixmp4backend(); make functions no-ops if False. - Import ixmp4 classes used only for hinting inside a TYPE_CHECKING block. - Add compose_maps() for common pattern of calling compose_*_maps(). - Remove redundant conditions in .core.Scenario, .model.MESSAGE.
- Parallel to configuration added in iiasa/ixmp#575.
- Use variable in install steps. - Expand documentation comment. - Temporarily add iiasa/ixmp4#171 branch for iiasa#894.
This began as a minimal PR to check that the message_ix test suite runs using ixmp4 storage (i.e. without the ixmp.JDBCBackend and underlying Java code).
It was then substantially expanded by @glatterf42 such that (most of) the message_ix test suite and all of the tutorials are confirmed to run using the IXMP4Backend.
Some things not in scope for this PR:
ixmpcode (including its test suite) tomessage_ix.Details
In order to achieve the testing, it:
the branch for Implement ixmp4 shim ixmp#552the ixmpmainbranch that:ixmp.testing.test_mpfixture to yield either the current ixmp.JDBCBackend-backed test Platform, or an ixmp4-backed test Platform.message_ix.testssuite, with either storage option. (message-ix-models and downstream/user code rely on this behaviour.)The branch is used by iiasa/message-ix-models#257.
Implementation/to dos:
ixmp.testing.test_mp, all tests inmessage_ix.teststhat use thetest_mpfixture will run twice: once with JDBCBackend, and once with ixmp4.How to review
PR checklist
Add, expand, or update documentation.See above.