-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add visualization of normative ranges #9
Conversation
@ashishsingh18 Note, I packaged the repository as a package. For this, renamed the |
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.
Please see the comments. Let's also discuss in the meeting.
if DataFile is None: | ||
self.data = None | ||
else: | ||
dio = DataIO() | ||
d = dio.ReadPickleFile(DataFile) | ||
self.SetData(d) | ||
|
||
if HarmonizationModelFile is None: | ||
self.harmonization_model = None | ||
else: | ||
dio = DataIO() | ||
m = dio.ReadPickleFile(HarmonizationModelFile) | ||
self.SetHarmonizationModel(m) |
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.
Data reading should happen outside this class. Please move this out to the presenter.
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.
Would this require adding following to the constructor of MainWindow
?
#read input data
dio = DataIO()
d = dio.ReadPickleFile(DataFile)
m = dio.ReadPickleFile(HarmonizationModelFile)
#set data in model
self.model.SetData(d)
self.set_harmonization_model(m)
#populate the ROI only if the data is valid
#Otherwise, show error message
if(self.model.IsValid()):
self.PopulateROI()
self.PopulateHue()
else:
QtWidgets.QMessageBox.critical(self,
'Error',
"Invalid Input Data. Please check the data and try again.",
QtWidgets.QMessageBox.Ok)
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.
Yes, we will need to make appropriate change in mainwindow, not necessarily the constructor of mainwindow though.
How about renaming the repo to iStagingTools ( without hypen ), that might work and may also reduce confusion between the other repo named 'iSTAGING'? |
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.
Looks good. We will handle the issues via separate PRs.
This implements normative ranges for data based on an external model if loaded. New menu item added for model file, e.g.
/comp_space/abdulkaa/MUSE_harmonization_model.pkl