Adjust to ixmp4 versions v0.16.0 and higher#640
Conversation
|
|
Beyond code, can you please give a clear indication what code was written by you, and which is output from your LLM(s)? This would help me detect when changes convey intent vs. not. I notice failures of:
I will also at some point rewrite the branch history to:
|
|
No actual LLM /code/ here, only that summary. I looked through the top level md files for guidelines like merge strategy and commit format, but I must have missed it. Do you have a link for future reference? |
|
Looks like i forgot about test_ixmp4_io.py and there was a comment that was too long. Mypy still fails however, I think due to a wrongly cached dependency. pytest.yaml:157 says "Comment this line to force clear". Is the idea that I push a commit with that line commented out? Unrelated to that, locally I still get a few errors: Most of them are in files I did not touch. One is because one of two specifiers in a mypy-ignore comment is unused, I added one specifier because the one that was already there did not catch my error. I assume that this has something to do with different mypy versions or the type hints were still a work-in-progress, so I will leave it like this. |
OK, thank for the details. Knowing such things really lowers the cognitive load of review, because I know what to treat as reliable vs. not.
We have a common set of pages that apply to message_ix, ixmp, message-ix-models, and message_data: https://docs.messageix.org/en/latest/contributing.html. CONTRIBUTING.rst should link to that URL.
These predate your PR, see e.g. here. I suspect (have not had time to check) these are due to a new release of pandas or pandas-stubs (which is not necessarily synced with pandas per se). I'd say these are also out of scope for this PR; I'll try to make another one to fix them, then we can rebase this branch.
You could, yes, but let's see if that happens as a result of the other things. |
Follows the recent refactor of ixmp4 and cleans up existing work to prefer using the "facade layer" (ixmp4.core) wherever possible. There are still a few missing features and uses of private functionality. I had an LLM summarize all the relevant comments as most of them were not written by me. I think many points are moot, but it might serve as a reference later:
Output
1. Upstream ixmp4 Features Needed
These are the gaps that look hard or wrong to solve purely in ixmp, because ixmp4.py is already compensating with placeholders, private API calls, or lossy behavior.
strictreads. ixmp4 currently appears to store only run-level meta, which blocks most of test_meta.py.lock,unlock, andrevert/discardAPI would remove fragile backend coupling.get_scenarios()andlast_update()need real created/updated/locked metadata instead of placeholders, as called out in ixmp4.py and ixmp4.py.synonymand partly discardsparentsemantics in ixmp4.py, and the related tests remain xfailed in test_platform.py and test_timeseries.py.list()were correctly run-scoped by default, ixmp would not need defensive filtering everywhere.clear_solution(from_year=...)cannot be faithfully implemented until ixmp4 exposes removal semantics beyond optimization-only state; see ixmp4.py.2. ixmp-Side Compatibility Shims
These are gaps that look feasible to improve inside ixmp without waiting on new ixmp4 APIs, though some are tradeoffs rather than perfect fixes.
schemeis already stored in run meta, soget_scenarios()can likely surface that immediately instead of"IXMP4Run"from ixmp4.py. The remaining audit fields could stay blank orNonerather than fake strings.first_model_yearclone modes with explicit exceptions instead of warning-and-continue behavior from ixmp4.py.run_id()semantics at the ixmp layer. If ixmp conceptually wants a stable run identifier rather than version, ixmp can map torun.idinstead ofrun.version, provided tests and callers are updated consistently. The current implementation in ixmp4.py looks like a compatibility shortcut, not a requirement.last_update()behavior in ixmp even before upstream support. ReturningNonewould be more honest than the invalid placeholder date currently shown in ixmp4.py.add_timeslice()andtimeslices()so tests and platform APIs behave more like JDBC, even if ixmp4 still only materializes actual usage through datapoints._find_item()at ixmp4.py.equ_listandvar_list, initialize missing items for MESSAGE-like scenarios, and then hand off to the existing reader. That would reduce the gap noted in ixmp4_io.py without requiring new ixmp4 primitives.NotImplementedErrorin narrow places. In a few methods, honest partial support is better than fabricated JDBC-shaped data.How to review
Read the diff and note that the CI checks all pass. Ensure old and new code snippets in encapsulated logic (i.e. functions) still expresses the same intent.
PR checklist