Manual adjustments for .engage.workflow migration
#351
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
khaeru
force-pushed
the
migrate/engage-tools-cleanup
branch
from
e370556 to
e20d9fd
Compare
khaeru
force-pushed
the
migrate/engage-tools-cleanup
branch
5 times, most recently
from
ae577d6 to
5b6958e
Compare
- Format docstrings according to code style, wrap at 88 characters. - Correct misspellings. - Add type hints. - Use Scenario.transact() where possible. - Use f-strings. - Use ScenarioInfo.Y instead of get_optimization_years(). - Use ScenarioInfo.N instead of get_nodes(). - Remove if __name__ == "__main__" blocks.
- Sort and adjust imports.
khaeru
force-pushed
the
migrate/engage-tools-cleanup
branch
from
5b6958e to
85bc5a3
Compare
4 tasks
- Adjust imports. - Type hint test functions.
- Use parametrized generics from default collections. - Import from collections.abc instead of typing.
- Copied from: https://github.com/iiasa/navigate-workflow - file definitions/scenario/scenarios.yaml - commit abb23814b7de3618ec4287ec014ddfce3e07c030 (2023-04-21) - Adjust code that reads this file to use the new location.
khaeru
force-pushed
the
migrate/engage-tools-cleanup
branch
from
85bc5a3 to
3156c69
Compare
- Use path_fallback in .navigate.wp2.utils. - Change load_private_data → load_package_data in .buildings.build.load_config().
khaeru
force-pushed
the
migrate/engage-tools-cleanup
branch
from
3156c69 to
0054ac1
Compare
khaeru
force-pushed
the
migrate/engage-tools-cleanup
branch
from
0054ac1 to
108a3b0
Compare
glatterf42
approved these changes
May 16, 2025
|
|
||
| from message_ix import Scenario | ||
|
|
||
| from message_ix_models import Context |
There was a problem hiding this comment.
I like all the new type hints :)
I'm just wondering: what's the benefit of importing these only if TYPE_CHECKING? Does this save actual runtime for the tests?
There was a problem hiding this comment.
Not sure if it has any performance effect.
I usually follow the heuristic "things used only for type checking in a module are imported in a TYPE_CHECKING block". The corollary is that the top-level imports contain only things actually referenced/called in the module.
This habit has helped me a number of times to avoid issues with circular imports.
Comment on lines
+87
to
+90
| This documentation and the related code was migrated from :mod:`message_data` ``dev`` | ||
| branch as of commit 8213e6c (2025-05-08) in :pull:`350`. | ||
| It does not reflect further changes made on the :mod:`message_data` ``main`` branch, | ||
| other related branches, or in forks or other repositories. |
There was a problem hiding this comment.
This is clear to me, but I might not be the best to judge this since I already understood this concept when I migrated the legacy reporting version.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This branch makes additional adjustments to the branch for #350 after the semi-automated migration process.
These include:
How to review
.engage.workflowmigration #351 in the preview build of the docs, and all the pages/functions it links to; ensure they clearly indicate the state of the code.PR checklist