-
Notifications
You must be signed in to change notification settings - Fork 13
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
Adding Save/Load Template to Routine Page #145
Conversation
…d combobox to call template loading
…or handling in load_template_yaml()
…current options as yaml template
testing save template
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.
Overall looks good to me. Just had a few comments.
Also, I don't think it should be a problem, but you might want to merge in the more recent commits from main.
@@ -98,6 +98,16 @@ def init_ui(self): | |||
vbox = QVBoxLayout(self) | |||
vbox.setContentsMargins(8, 8, 8, 8) | |||
|
|||
# Load Template Button |
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.
I'm guessing the button definitions are in different places because the actual buttons themselves are as well? Maybe you can show us tomorrow.
@@ -98,6 +100,10 @@ def __init__(self): | |||
# Trigger the re-rendering of the environment box | |||
self.env_box.relative_to_curr.setChecked(True) | |||
|
|||
# Template path | |||
# template_dir = os.path.join(self.BADGER_PLUGIN_ROOT, "templates") | |||
self.template_dir = "/home/physics/mlans/workspace/badger_test/Badger/src/badger/built_in_plugins/templates" |
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.
I think what you have commented is the right idea.
@wenatuhs Do we want to keep it like this, or do you want to add a templates directory to your copy of Badger on prod?
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.
I agree that we should go the commented way (template_dir = os.path.join(self.BADGER_PLUGIN_ROOT, "templates")
), since templates are specific to the applications, so do the plugins, putting templates under plugins feels like the intuitive way.
@@ -168,7 +186,9 @@ def init_ui(self): | |||
# vbox.addWidget(group_meta) | |||
|
|||
# Env box | |||
BADGER_PLUGIN_ROOT = config_singleton.read_value("BADGER_PLUGIN_ROOT") | |||
self.BADGER_PLUGIN_ROOT = BADGER_PLUGIN_ROOT = config_singleton.read_value( |
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.
Honestly, you might even just stick this in the constructor since this isn't really a UI thing.
Tried rebasing the PR onto the main branch and it worked nicely, a few more things to consider:
All of the above can wait for the next PR though :) |
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.
Thanks for the excellent work! I see no issues merging this into main :)
Introduces new features to routine page, with buttons to load and save templates for optimization runs.