search

UCL / SkullBaseNavigation

Other
2 stars 1 forks source link

Add save button - [merged] #93

Closed tdowrick closed 3 years ago

tdowrick commented 3 years ago

In GitLab by @RolandGuichard on Jun 10, 2019, 11:21

_Merges add-save-button -> startdeps

Fixes #56

tdowrick commented 3 years ago

In GitLab by @RolandGuichard on Jun 10, 2019, 11:23

Hi @AnastasisGeorgoulas and @ThomasDowrick could you please review this ? After reviewing, I am thinking of coming to CBH later this week for a test.

tdowrick commented 3 years ago

In GitLab by @AnastasisGeorgoulas on Jun 10, 2019, 17:01

Commented on skullbasenavigation/sbn_slicelet.py line 352

I'm not a huge fan of dots in filenames because it can cause confusion with extensions, but I'm not sure what's a good alternative here (":" is not a valid character for Windows file names). Maybe have the whole time part without separators (eg 180110)? But that's not too clear either.

tdowrick commented 3 years ago

In GitLab by @AnastasisGeorgoulas on Jun 10, 2019, 17:03

Commented on skullbasenavigation/sbn_slicelet.py line 364

It's safer to create the path using the standard library, which will work regardless of whether the directory name ends in a / or not.

tdowrick commented 3 years ago

In GitLab by @AnastasisGeorgoulas on Jun 10, 2019, 17:05

Commented on skullbasenavigation/sbn_slicelet.py line 370

I'm a little confused about the new argument; is the intention to ever pass a specific timestamp other than the current one?

tdowrick commented 3 years ago

In GitLab by @AnastasisGeorgoulas on Jun 10, 2019, 17:07

Commented on skullbasenavigation/config.py line 46

I would maybe put an / here for consistency, but using the os.path module would make this a moot point.

tdowrick commented 3 years ago

In GitLab by @RolandGuichard on Jun 10, 2019, 19:08

Commented on skullbasenavigation/sbn_slicelet.py line 364

Yes, that's what I thought too when writing this. I wanted it to be consistent wiht the save_transforms one though.

tdowrick commented 3 years ago

In GitLab by @RolandGuichard on Jun 10, 2019, 19:10

Commented on skullbasenavigation/sbn_slicelet.py line 370

No, actually save_transforms has 2 callers: the Save Transforms button and the Save All button, which saves both the scene and the transform. In that case, I wanted the timestamp to be the same for both files.

tdowrick commented 3 years ago

In GitLab by @RolandGuichard on Jun 10, 2019, 20:07

Commented on skullbasenavigation/sbn_slicelet.py line 364

changed this line in version 2 of the diff

tdowrick commented 3 years ago

In GitLab by @RolandGuichard on Jun 10, 2019, 20:07

added 1 commit

Compare with previous version

tdowrick commented 3 years ago

In GitLab by @RolandGuichard on Jun 10, 2019, 20:07

Commented on skullbasenavigation/sbn_slicelet.py line 364

Done

tdowrick commented 3 years ago

In GitLab by @RolandGuichard on Jun 10, 2019, 20:08

Commented on skullbasenavigation/sbn_slicelet.py line 352

Hm yeah, not convinced either for a better solution.

tdowrick commented 3 years ago

In GitLab by @RolandGuichard on Jun 10, 2019, 20:15

Commented on skullbasenavigation/config.py line 46

changed this line in version 3 of the diff

tdowrick commented 3 years ago

In GitLab by @RolandGuichard on Jun 10, 2019, 20:16

added 1 commit

Compare with previous version

tdowrick commented 3 years ago

In GitLab by @RolandGuichard on Jun 10, 2019, 20:16

Commented on skullbasenavigation/config.py line 46

OK, done.

tdowrick commented 3 years ago

In GitLab by @ThomasDowrick on Jun 11, 2019, 08:36

Commented on skullbasenavigation/sbn_slicelet.py line 370

We shouldn't need to call save_transforms from the Save All button, as the saveScene function should save the transforms anyway.

tdowrick commented 3 years ago

In GitLab by @ThomasDowrick on Jun 11, 2019, 08:42

@RolandGuichard You should be able to check this is working on your local machine, by loading some data into Slicer and then checking that it has been saved properly. You're fine to come over to CBH later in the week if you want to (I can check the OR schedule), but probably not necessary.

Separately, we have the demo to surgeons on the 29th, so we should run through the software, either late next week or at the start of the following week, with whatever extra changes we have made by then.

tdowrick commented 3 years ago

In GitLab by @RolandGuichard on Jun 11, 2019, 09:03

Hi @ThomasDowrick that's right. I checked that it worked. It was more for the sake of safety. But I agree that it might sound like an overkill.

tdowrick commented 3 years ago

In GitLab by @ThomasDowrick on Jun 11, 2019, 09:13

Cool. I can probably just go downstairs and give it a quick check when you're happy with it, if that is easier.

tdowrick commented 3 years ago

In GitLab by @RolandGuichard on Jun 11, 2019, 09:14

Yeah OK. Thanks.

tdowrick commented 3 years ago

In GitLab by @RolandGuichard on Jun 11, 2019, 14:24

Commented on skullbasenavigation/sbn_slicelet.py line 370

changed this line in version 4 of the diff

tdowrick commented 3 years ago

In GitLab by @RolandGuichard on Jun 11, 2019, 14:24

added 3 commits

Compare with previous version

tdowrick commented 3 years ago

In GitLab by @ThomasDowrick on Jun 19, 2019, 10:32

added 2 commits

Compare with previous version

tdowrick commented 3 years ago

In GitLab by @ThomasDowrick on Jun 19, 2019, 10:32

merged