Conversation
When using `read_base()` context, we can override variables in the base config file by: ```python with read_base(foo=1): from ..base import foo ``` Now `foo` in the base will be overriden to 1.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces experimental functionality to override variables in base configuration files when using the read_base() context manager. The implementation allows passing key-value pairs to read_base() that will override corresponding variables in the imported base modules.
Key changes:
- Added support for keyword arguments in
read_base()to specify variable overrides - Implemented AST transformation to apply overrides during config parsing
- Updated test cases to validate the new override functionality
Reviewed Changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| mme/config/config.py | Core implementation of override functionality in config parsing |
| mme/config/utils.py | Added OverideAssignTransformer class for AST transformation |
| tests/test_config/test_base_modification.py | Updated test to validate override behavior |
| tests/data/config/lazy_module_config/test_change_base_var.py | Modified test config to use override syntax |
| tests/data/config/lazy_module_config/test_base.py | Extended base config with additional variables |
| pyproject.toml | Updated Python version support and dependencies |
| mme/init.py | Version bump to 0.10.10 |
| tests/test_config/test_lazy.py | Removed deprecated numpy.compat imports |
| tests/test_config/test_config_legacy.py | Simplified temporary directory test logic |
| tests/data/config/lazy_module_config/test_ast_transform.py | Removed deprecated numpy.compat import |
| .github/workflows/test.yml | Added codecov integration |
| .github/workflows/lint.yml | Updated Python setup action and added pre-commit formatting |
| .github/pull_request_template.md | Removed entire PR template content |
| return node | ||
|
|
||
|
|
||
| class OverideAssignTransformer(ast.NodeTransformer): |
There was a problem hiding this comment.
Class name contains a typo: 'Overide' should be 'Override'.
| class OverideAssignTransformer(ast.NodeTransformer): | |
| class OverrideAssignTransformer(ast.NodeTransformer): |
| from .utils import ( | ||
| ConfigParsingError, | ||
| ImportTransformer, | ||
| OverideAssignTransformer, |
There was a problem hiding this comment.
Import name contains a typo: 'OverideAssignTransformer' should be 'OverrideAssignTransformer'.
| OverideAssignTransformer, | |
| OverrideAssignTransformer, |
| modified_code, filename=filename | ||
| ) | ||
| if override_vars: | ||
| transform2 = OverideAssignTransformer(override_vars) |
There was a problem hiding this comment.
Class instantiation contains a typo: 'OverideAssignTransformer' should be 'OverrideAssignTransformer'.
| transform2 = OverideAssignTransformer(override_vars) | |
| transform2 = OverrideAssignTransformer(override_vars) |
| # must be SSA | ||
| if len(node.targets) != 1: | ||
| return node | ||
| if (key := node.targets[0].id) in self.override_vars: |
There was a problem hiding this comment.
This code assumes node.targets[0] has an id attribute, but it could be other types like ast.Subscript or ast.Attribute. This will raise an AttributeError for assignments like x[0] = 1 or obj.attr = 1.
| if (key := node.targets[0].id) in self.override_vars: | |
| if isinstance(node.targets[0], ast.Name) and (key := node.targets[0].id) in self.override_vars: |
| except Exception as e: | ||
| raise SyntaxError( | ||
| except ImportError: | ||
| warnings.warn("formater 'black' is not found.") |
There was a problem hiding this comment.
Typo in warning message: 'formater' should be 'formatter'.
| warnings.warn("formater 'black' is not found.") | |
| warnings.warn("formatter 'black' is not found.") |
When using
read_base()context, we can override variables in the base config file by:Now
fooin the base will be overriden to 1.