In my opinion, the functionality that we want to keep should also have unit tests to (1) indicate that it is maintained and (2) to demonstrate how to use it.
- Queries
- All Stories
- Search
- Advanced Search
- Transactions
- Transaction Logs
All Stories
Sep 30 2020
I found a way to solve this, similar to T27721: [Segmentation] 2D Fast Marching crashes with 4D image and static segmentation: By transforming the time step from the reference image to the working image with
TimePointType referenceImageTimePoint = referenceImage->GetTimeGeometry()->TimeStepToTimePoint(t); TimeStepType workingImageTimeStep = workingImage->GetTimeGeometry()->TimePointToTimeStep(referenceImageTimePoint);
in mitk::LiveWireTool2D::ConfirmSegmentation().
However, inside mitk::LiveWireTool2D there are a lot of uses of positionEvent->GetSender()->GetTimeStep() which all lead to a wrong timestep (e.g t = 2, although a static segmentation only has 1 timestep = 0).
That means:
- I'm not sure about any negative side-effects which I don't see with my manual testing right now
- There is a basic flaw in the design which should be discussed instead of just fixing the symptoms with the suggested fix
The idea is to raise an error if the user tries to extract a task that is not contained in the set of tasks.
mitkToolInteractionTestSuite in mitkToolInteractionTest.cpp does not contain any tests (all disabled)
In T27282#211185, @eisenman wrote:The function testThenRank has an argument FUN that is set to "significance" internally. The value that is actually passed is not used.
@wiesenfa Should "significance" be the default value in the signature already or should the argument be removed?
The same problem exists in T27800: [Segmentation] Live Wire crashes with 4D image and static segmentation: Here, mitk::SegTool2D::GetAffectedImageSliceAs2DImageis called with a reference image (the segmentation) inside mitk::LiveWireTool2D::ConfirmSegmentation() with t != 0.
I just found out that the same code (mitk::SegTool2D::GetAffectedImageSliceAs2DImage) is used when using e.g. the simple add-tool. However, here unsigned int timeStep = positionEvent->GetSender()->GetTimeStep(image); returns 0 as timestep, although the timestep was set to something != 0 in the image navigator. This works, because mitk::BaseRenderer::GetTimeStep already transforms the current timepoint with data->GetTimeGeometry()->TimePointToTimeStep(GetTime());.
Sep 29 2020
Maybe it's possible to change
I tried to find a solution. The problem is the same problem we had with other segmentation operations:
mitk::SegTool2D::GetAffectedImageSliceAs2DImage is called with a reference image (the segmentation) and a timestep although the segmentation is static and the timestep != 0 does not exist.
So obviously one would need to translate a timepoint into the timestep of the static segmentation. But I don't see a way to access the current timepoint and also the whole 2D fast marching is totally unclear to me.
mitk::FastMarchingTool already holds the current timestep, which is changed by listening to the image navigator. But the timestep does not help us here.
In T26975#208105, @thomass wrote:I can partly confirm it.
- The segmentation is performed on the current selected timestep (if you switch to timestep 2 --> segmentation occurs on timestep 2 = CORRECT; maybe fixed with the task!?)
In T27096#211175, @kalali wrote:I will land the current documentation onto develop now. Open questions still remain but for me the whole plugin is chaotic and therefore I don't want to dive deeper into it before discussing the current status.
The "Labels" tab does not even show the widgets it's supposed to show. Lots of functions are not implemented, functionality is not consistent :|
I'm was able to land the differential. I simply merged the branch, after I updated D255 with it. Everything seems fine but I'm not sure if old changes of the documentation have also been re-added. Please check (hint could be the removed "The" in front of the documentation page names).
The function testThenRank has an argument FUN that is set to "significance" internally. The value that is actually passed is not used.
@wiesenfa Should "significance" be the default value in the signature already or should the argument be removed?
Found the bug in CDash. In app/Model/AuthToken.php:51 the "special value" 9999-12-31 23:59:59 is inserted into the database whereas the maximum value for TIMESTAMP is 2038-01-19 03:14:07. Hence the "special value" appears as 0000-00-00 00:00:00 in the database which will trigger the deletion when it is compared with the current date.
I will land the current documentation onto develop now. Open questions still remain but for me the whole plugin is chaotic and therefore I don't want to dive deeper into it before discussing the current status.
Sigh. Just one time I'd like not to stumble over bugs when dealing with CDash. I implemented the authorization, double-checked that the proxy does not throw away the authorization HTTP header, watched the cdash database, and figured out that as soon as CDash is comparing a valid token... it deletes it from the database.
Deleted branch feature/T25882-CDashAuthorization.
Consensus explanations to be updated after input request.
Pushed new branch feature/T25882-CDashAuthorization.