frontend focused load raw scan from a folder#57
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #57 +/- ##
==========================================
+ Coverage 82.51% 85.61% +3.10%
==========================================
Files 6 7 +1
Lines 223 278 +55
==========================================
+ Hits 184 238 +54
- Misses 39 40 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| event_type = type(event) | ||
| if callable_list := self.registry.get(event_type): | ||
| for callable in callable_list: | ||
| callable(event) |
There was a problem hiding this comment.
Like I was saying at status, there are a couple more features outlined in the EventBroker ticket that the prototype did not implement. You can view my additions here. I still need to write tests for it.
Seeing as the event broker is to be implemented in a different ticket, could you mark this as a stub implementation? I dont see any unit tests written for this explicitly, which is fine, that will be added as part of the event broker ticket.
|
We may need to update the pr template to include a link to the ticket the work is associated with. |
Functional Review No. 1Review according to the Acceptance Criteria in the story: Implement the design outlined in the diagrams below
❌ The text in the File Menu of the tool bar needs to be updated to match the requirements. Please refer to Balsamiq
I clicked "Load Folder" assuming that is the menu option that needs to be renamed.
❌ This did not occur. The AC in the DOORS page is a little more explicit as to what this means, though Im not sure this is captured in Balsamiq, maybe reach out to John/Andrei for clarification. I understand that the backend isnt implemented but there should be a stubbed implementation to show off this feature
Inspecting the code I can confirm that the qdialogue output is actually sent to the model |
For returning the filenames, I feel the original story should probably push this to be part of backend epic story. As we need to determine how do we want to organize the data, IPTS->exp->data or exp->IPTS->data. Maybe we can even start with facility as ORNL. These probably worth a chat with CIS. |
There still needs to be a pop up that says which files youre about to attmept to load, at least that's what I interpret from the acceptance criteria. I did not see an empty version of that. |
|
|
||
|
|
||
| @Singleton | ||
| class TaviProject(TaviProjectInterface): |
There was a problem hiding this comment.
I think we should generally add a suffix to denote the kind of object it is.
Not gonna hold up merging over it, but I would like to name this TaviProjectModel for example.
Function Review 2In light of the conversation on slack with John and Andrei, the AC has been adjusted.
✔️ Passes my function review |
|
Feel free to merge when ready. |
3 participants
Short description of the changes:
This PR sets up the front end view widgets and a pop up window called "Select a folder" to load tavi data from a folder.
Manual test for the reviewer
To test, click on "File" -> "Load Folder", select any random folder click "choose". The pop up window should close, console should print "folder directory received...." and "sent to worker!". This means the MVP connect is set up correctly and that the connected load function was sent to different thread than main GUI.