Dhanmoni gsoc21 toppview #5720
Dhanmoni gsoc21 toppview #5720
Conversation
…end to JS using QJsonDocument
…end to JS instead of looping through data each time we click on sequence, some debug messages removed
…essions if we have multiple of them; code cleanup
…i/OpenMS into gui/proteintable
Some cleanup
Fixes, cleanups and todos
| @@ -1488,6 +1501,10 @@ namespace OpenMS | |||
| { | |||
| if (!ws_.currentSubWindow()) | |||
| { | |||
| if (activeSubwindowID_ < (Size) ws_.subWindowList().size()) | |||
There was a problem hiding this comment.
i'm not sure what this is doing...
if the subwindow is currently empty, why return the "old" one?
What happens if the window was closed?
There was a problem hiding this comment.
I think it had something to do with focus.
If the plot area is not focussed, you get a nullptr for currentSubwindow.
But I think some other code depends on getting information from the currently visible (but not active)
spectra plot.
Not sure if I explained that well.
| 'Content-Type': 'application/json' | ||
| } | ||
| }) | ||
| if (!response.ok) { |
There was a problem hiding this comment.
what if ebi website is down?
Then it throws an error. Ok.
But what does that mean for the interface? Is it a messagebox or does it close the app?
There was a problem hiding this comment.
I think it leaves the "browser" window half loaded without the sequence and throws an error in the "website log" (i.e. in the "Chrome tools" or equivalent view)
| void SpectraTreeTab::itemSelectionChange_(QTreeWidgetItem* current, QTreeWidgetItem* previous) | ||
| { | ||
| /* test for previous == 0 is important - without it, | ||
| the wrong spectrum will be selected after finishing | ||
| the execution of a TOPP tool on the whole data */ | ||
| if (current == nullptr || previous == nullptr) | ||
| // WUT???? | ||
| if (current == nullptr) |
There was a problem hiding this comment.
has this been verified?
The test for previous was there for a reason?!
There was a problem hiding this comment.
Not really, but this cannot be the solution to whatever problem this is trying to solve.
With this, you cannot ever set a selection if nothing was selected previously.
There was a problem hiding this comment.
The problem was, if i remember correctly, that TOPPView crashed.
Hence the extra check.
So now it crashes again, given the right circumstances, but we can set a selection?
Seems like this needs some investigation...
There was a problem hiding this comment.
Yes true, I think whatever made it crash should probably be fixed instead of restricting other functions just to be safe.
@timosachsenberg can you quickly check? You have it built already.
There was a problem hiding this comment.
sure what exactly?
There was a problem hiding this comment.
Didn't crash now but still a lot of strange stuff happening when e.g., switching between ID and spectra view, running tools on them etc. :) also not sure how reproducible that crash was (but I am pretty sure that there was a crash as @cbielow pointed out)
There was a problem hiding this comment.
hehe no still :) the usual strange stuff in TOPPView
There was a problem hiding this comment.
Ok yes, I remember still having issues as well.
src/openms_gui/source/VISUAL/TVIdentificationViewController.cpp
Outdated
Show resolved
Hide resolved
| j["accession_num"] = accession_num; | ||
| j["protein_sequence_data"] = pro_seq; | ||
| j["peptides_data"] = pep_data; | ||
| backend_.m_json_data_obj_ = j; |
There was a problem hiding this comment.
can this be a move? or not necessary because "QJsonObject is an implicitly shared class, and shares the data with the document it has been created from as long as it is not being modified."
|
Thanks! |
Description
supersedes #5489 and adds a button to switch between horizontal and vertical table layout
Checklist:
How can I get additional information on failed tests during CI:
If your PR is failing you can check out
Note:
Advanced commands (admins / reviewer only):