-
Notifications
You must be signed in to change notification settings - Fork 35
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
Packing GUI #308
base: 1.x
Are you sure you want to change the base?
Packing GUI #308
Conversation
test file
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 didn't look into the tree yet, but it looks like you should be holding some more state about the current configuration, rather than relying on the configuration file, and non-obvious global state such as flag
, numberOfRuns
, runName
.
I think it might make sense to keep to usual stack-based procedures, with AddRunWindow
being your base state, calling TraceWindow
when a new run is to be added, calling editConfigurationWindow
when the user is ready to move on to that stage, and have editConfigurationWindow
call similarly into renameFileWindow
and AddFilesWindow
. And remove this switch_frame()
method that complicates things for you. That way those objects don't cease to exist during their natural lifetime and can keep state.
reprozip-gui/gui.py
Outdated
data = yaml.safe_load(fr) | ||
|
||
data['inputs_outputs'][self.index]['name'] = self.fileNameEntry.get() | ||
with open(self.master,filepath, "w") as fw: |
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.
.filepath
here 😉
reprozip-gui/gui.py
Outdated
with open(self.master.filepath) as fr: | ||
data = yaml.safe_load(fr) | ||
|
||
if(flag != 3): |
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.
This is really a hack. You shouldn't be passing this through the attribute on the application, rather it should be set by TraceWindow
before switching.
# Run 0 being the first run | ||
numberOfRuns = 0 | ||
#to keep a check on which was the last window visited, -1 saying no widnows prior | ||
flag = -1 |
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.
This is only used by AddRunWindow
to set the run name, should get rid of it
This is a draft. PR opened for code review.
This is a GUI allowing users to trace, edit the config, and pack easily.
cc @BinalModi
see #257