Skip to content

Conversation

@k1nshuk
Copy link

@k1nshuk k1nshuk commented Aug 11, 2025

This branch was created from Micah's v2.0.0 branch and has certain PlexosExporter specific changes to allow for the successful creation of the XML file. It also updates minimum dependency versions of PlexosDB to ensure it works with the r2x-plexos plugin.

@k1nshuk k1nshuk requested review from Copilot, mcllerena and pesap and removed request for mcllerena and pesap August 11, 2025 21:51
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR merges PlexosExporter-specific changes into the R2X v2.0.0 branch, implementing a plugin manager architecture that replaces the previous hardcoded model registry system. The changes update minimum dependency versions and refactor the codebase to use a centralized plugin management system.

  • Introduces a new PluginManager class for centralized registration and discovery of parsers, exporters, and system modifiers
  • Refactors the configuration system to use the plugin manager instead of hardcoded model lists
  • Updates dependency versions for PlexosDB and infrasys to ensure compatibility

Reviewed Changes

Copilot reviewed 43 out of 44 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
user_dict.yaml Adds example configuration file showing different ways to configure scenarios
tests/test_*.py Updates test files to use new plugin manager interface and filter functions
src/r2x/plugin_manager/*.py New plugin manager implementation with interfaces, defaults, and utilities
src/r2x/runner.py Refactored to use plugin manager for loading parsers, exporters, and system modifiers
src/r2x/config_*.py Updates configuration handling to use plugin manager instead of hardcoded enums
src/r2x/plugins/*.py Updates existing plugins to register with the plugin manager
src/r2x/parser/*.py Updates parsers to register CLI arguments and use new interfaces
src/r2x/exporter/*.py Updates exporters to use new plugin manager system
pyproject.toml Updates dependency versions for plexosdb and infrasys

k1nshuk and others added 2 commits August 11, 2025 15:59
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: pesap <pesap@users.noreply.github.com>
Copy link
Collaborator

@pesap pesap left a comment

Choose a reason for hiding this comment

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

LGTM overall, just that question before we can merge.


class PlexosConfig(BaseModelConfig):
"""Plexos specific configuration."""
model_config = {"protected_namespaces": ()}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need this?

Copy link
Author

Choose a reason for hiding this comment

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

Pydantic's recent versions has namespaces like model_* protected by default. So not having that was resulting in warnings. I cannot recall if there had been validation errors, but I definitely remember the warnings. That statement basically disables checking for all protected namespaces. An alternative, if we want to retain some namespace checking would be the following

Suggested change
model_config = {"protected_namespaces": ()}
model_config = {"protected_namespaces": ("settings_", "json_", "schema_")}

or we stop using names with model_ and just use name and year for attributes. The relevant namespaces that I can find are

model.model_dump()
model.model_validate()
model.model_fields
model.model_config
model.json_schema()
model.schema_json() 

k1nshuk and others added 2 commits September 11, 2025 14:05
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Kinshuk Panda <kinshukpanda@gmail.com>
@mcllerena mcllerena merged commit 0c4ff00 into NatLabRockies:v2.0.0 Sep 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants