-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Support environment variables in config #1565
base: master
Are you sure you want to change the base?
Conversation
Hi @innerlee , you can merge the upstream master to your branch to fix the error of building documentation |
https://mmdetection.readthedocs.io/en/latest/1_exist_data_model.html Can we use the Line 550 in 44e7eee
|
We also need to update the usage of Config (https://mmcv.readthedocs.io/en/latest/understand_mmcv/config.html) and add some unit tests for the feature. |
@@ -176,7 +196,9 @@ def _substitute_base_vars(cfg, base_var_dict, base_cfg): | |||
return cfg | |||
|
|||
@staticmethod | |||
def _file2dict(filename, use_predefined_variables=True): | |||
def _file2dict(filename, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add argument use_environment_variables
for fromfile
function, as it call _file2dict
Hi @innerlee , is there any progress? |
Sorry being busy. Will complete it when I got time |
a09bcdc
to
9663996
Compare
Signed-off-by: lizz <[email protected]>
Signed-off-by: lizz <[email protected]>
assert cfg.item88 == 'OK' | ||
|
||
for file in ['tt.py', 'tt.json', 'tt.yaml']: | ||
with pytest.raises(Exception): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with pytest.raises(Exception): | |
with pytest.raises(ValueError, match='...'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding the match argument here will help us know why it should raise an exception.
Hi, is any progess? |
Motivation
Use environment variable to dynamically change parts of the config.
Kind of a template functionality.
Modification
Please briefly describe what modification is made in this PR.
BC-breaking (Optional)
no breaking change.
Use cases (Optional)
If you want to test many datasets on a checkpoint, and the only difference is the dataset path, then you can use environment variable as a placeholder. No need to create many configs anymore.
Before:
cfg1.py
cfg2.py
After:
cfg.py
Checklist
Tests and docs will be added later. Lets see the general reaction first.
Before PR:
After PR: