Skip to content
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

Remove dependency on deprecated Slicer core modules #168

Open
lassoan opened this issue Apr 30, 2020 · 14 comments
Open

Remove dependency on deprecated Slicer core modules #168

lassoan opened this issue Apr 30, 2020 · 14 comments

Comments

@lassoan
Copy link
Contributor

lassoan commented Apr 30, 2020

We are planning to remove Slicer core modules that have been superseded by other modules.
https://discourse.slicer.org/t/removing-legacy-modules-in-slicer5/11371

We have noticed that this extension currently uses legacy editor module, instead of the current Segment Editor module.

If you want your extension to be available for the upcoming Slicer5 release, please remove dependency to deprecated modules. If you have any questions or need any help with the upgrade, you can write here or post to the Slicer forum. Thank you for your contribution.

@lassoan
Copy link
Contributor Author

lassoan commented May 2, 2020

@fedorov I'll try to upgrade this extension to make it use Segment Editor, but I would need a test data set that I can use. Could you send a link to at least one typical data set?

@fedorov
Copy link
Member

fedorov commented May 4, 2020

Andras, a sample dataset is in https://github.com/SlicerProstate/SliceTracker/releases/edit/test-data in Preop-deid. mpReview requires the data to first be sorted, and I've just uploaded a sorted dataset to make it easier for you into Preop-deid_sorted.zip. I tested with today nightly, and confirm it works - all you should do is install mpReview and its dependency, and when you start the module, point it to the unzipped content of the file above. Let me know if you have any questions. Thanks again for your help!

@lassoan
Copy link
Contributor Author

lassoan commented May 31, 2020

I get this error when trying to open the review form:

Traceback (most recent call last):
  File "C:/D/mpReview/mpReview.py", line 688, in onPIRADSFormClicked
    self.webView = qt.QWebView()
AttributeError: module 'qt' has no attribute 'QWebView'

QWebView has been removed from Qt quite a long time ago, so it is surprising that this module has been working with recent Slicer versions.

Does master branch of this repository contain the latest version? Which Slicer version have you tested it with?

@fedorov
Copy link
Member

fedorov commented May 31, 2020

The functionality you are trying to use has not been used in a long time. We use this module for clinical data, and we cannot use that form for clinical data. So I think it is just that this error has not been noticed. We can just remove those buttons altogether.

@lassoan
Copy link
Contributor Author

lassoan commented May 31, 2020

No problem at all, I just wanted to make sure I work on the correct version.

I followed the steps described in the Readme file but I'm not sure what happens after segmentation. Could you describe or do a screen recording of a complete analysis on the sample data set?

@fedorov
Copy link
Member

fedorov commented May 31, 2020

I tried to record the video, but discovered a new issue (looks like related to Editor), which I think is the cause of segmentations not loading back:

image

In any case, the screencast is here: https://www.screencast.com/t/tZcHPDe1U6f.

What should be happening after segmentation, the results are saved into the proper locations in the data folder, and when the user returns to the segmentation they can reload the previously created segmentations. I don't know if you have any runtime issues with your version of Slicer (I was testing with May 12 or so).

Does this help?

@lassoan
Copy link
Contributor Author

lassoan commented Jun 1, 2020

Yes, it helped, thank you.

What is the purpose of "Translate selected label map"? How often do you use it? Should it move one segment or all the segments?

I see some label propagation feature in the code but it does not show up in the GUI. Is it still used?

@fedorov
Copy link
Member

fedorov commented Jun 2, 2020

Both of those features were added by a group in Chicago that was using this module. We never had use for those features for our purposes. I wouldn't recall how exactly translate selected label map should function, and the guys at Chicago had about 3 engineers come and go (with only the first one making a significant contribution), so there is no one to answer questions on their side.

I think it is ok to move this feature out from the GUI, and add it later if anyone asks for it.

@lassoan
Copy link
Contributor Author

lassoan commented Jun 9, 2020

@fedorov I'm making progress. Can you upload an annotated data set with segmentations and fiducials so that I can test that it can be loaded correctly? Thank you.

@lassoan
Copy link
Contributor Author

lassoan commented Jun 11, 2020

@fedorov What is the end goal of the module? Segmentation of selected series and a list of target points? I just want to make sure I understand the module fully and don't accidentally remove features as I rework things.

@fedorov
Copy link
Member

fedorov commented Jun 12, 2020

Andras, yes, the end goal is segmentations (for calculating region-based image features, summary statistics, procedure planning) and/or list of fiducials (for biopsy targeting planning).

Since you mention you rework things, note that initially we used an approach where Slicer plugins were used to build the tree with NRRD reconstructions of series. Later I switched to using dcm2niix, and for the most part it is using NIfTI, but NRRD support should still be there.

Overall, going forward, it will probably make sense instead of building those reconstructions to switch to using DICOM database directly, perhaps with some cache of which plugins should be used. When I started with this module, DICOM database was too slow, but maybe now with all the improvements you did it is ok. But this is not for now of course.

About the example, I don't have any public dataset other than the one I shared. If you have a branch that you can share for testing, I can try it out for some of the non-deidentified datasets we use internally. Should we do that?

@lassoan
Copy link
Contributor Author

lassoan commented Jun 12, 2020

Can you process the data set that you have already shared so that it has all the outputs?

@fedorov
Copy link
Member

fedorov commented Jun 12, 2020

I uploaded another zip file that includes segmentations: https://github.com/SlicerProstate/SliceTracker/releases/download/test-data/Preop-deid_sorted_with_segmentations.zip.

The fiducials saving capabilitiy is used as part of the SliceTracker workflow. I did not figure out how to used it in standalone module. We don't use the latest SliceTracker in biopsy procedures, and I have to say I don't even know if the current version of SliceTracker works with the current version of mpReview. I have not been able to make time to keep things up to date, test and follow changes in Slicer core. Let's just do the segmentations for now.

@fedorov
Copy link
Member

fedorov commented Jun 13, 2020

I forgot to mention - you will need to make sure your user name in mpReview is set to "user" - that's the user name I used for the sample dataset. It will only show segmentations that match user name. User name is in Slicer.ini under [mpReview].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants