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

Add SlicerIGSpineDeformity extension #1893

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

SenonETS
Copy link

@SenonETS SenonETS commented Oct 15, 2022

Distribute the custom extension SlicerIGSpineDeformity

New extension

  • Extension has a reasonable name (not too general, not too narrow, suggests what the extension is for)
  • Repository name is Slicer+ExtensionName
  • Repository is associated with 3d-slicer-extension GitHub topic so that it is listed here. To edit topics, click the settings icon in the right side of "About" section header and enter 3d-slicer-extension in "Topics" and click "Save changes". To learn more about topics, read https://help.github.com/en/articles/about-topics
  • Extension description summarizes in 1-2 sentences what the extension is usable (should be understandable for non-experts)
  • Any known related patents must be mentioned in the extension description.
  • If source code license is more restrictive for users than BSD, MIT, Apache, or 3D Slicer license then the name of the used license must be mentioned in the extension description.
  • Extension URL and revision (scmurl, scmrevision) is correct, consider using a branch name (main, release, ...) instead of a specific git has to avoid re-submitting pull request whenever the extension is updated
  • Extension icon URL is correct (do not use the icon's webpage but the raw data download URL that you get from the download button - it should look something like this: https://raw.githubusercontent.com/user/repo/main/SomeIcon.png)
  • Screenshot URLs (screenshoturls) are correct, contains at least one
  • Homepage URL points to valid webpage containing the following:
    • Extension name
    • Short description: 1-2 sentences, which summarizes what the extension is usable for
    • At least one nice, informative image, that illustrates what the extension can do. It may be a screenshot.
    • Description of contained modules: at one sentence for each module
    • Tutorial: step-by-step description of at least the most typical use case, include a few screenshots, provide download links to sample input data set
    • Publication: link to publication and/or to PubMed reference (if available)
    • License: We suggest you use a permissive license that includes patent and contribution clauses. This will help protect developers and ensure the code remains freely available. We suggest you use the Slicer License or the Apache 2.0. Always mention in your README file the license you have chosen. If you choose a different license, explain why to the extension maintainers. Depending on the license we may not be able to host your work. Read here to learn more about licenses.
    • Content of submitted s4ext file is consistent with the top-level CMakeLists.txt file in the repository (description, URLs, dependencies, etc. are the same)

@ungi
Copy link
Contributor

ungi commented Oct 15, 2022

Hi, I wanted to test the module in this extension called "sl_01__LaminaLandmark_Labeling", but I get this error message when I try to add it to my Slicer:

  File "E:/SlicerIGSpineDeformity/sl_01__LaminaLandmark_Labeling/sl_01__LaminaLandmark_Labeling.py", line 11, in <module>
    from PyQt5.QtWidgets import QMessageBox, QFileDialog
ModuleNotFoundError: No module named 'PyQt5'

Could you please add instructions to the repository README file on how to install the extension and its dependencies?
If this pull request is merged, will Slicer find PyQt5? I'm not too familiar with that library, but I thought it's not part of Slicer.
Thank you.

@jamesobutler
Copy link
Contributor

Yes Slicer uses PythonQt and not PyQt. Syntax is similar, but not exact. QFileDialog would be available in Slicer at

import qt
qt.QFileDialog

@lassoan
Copy link
Contributor

lassoan commented Oct 16, 2022

Probably you could pip install PyQt, but I would not recommend it, because PyQt's free license is a restrictive viral license (GPLv3). We don't explicitly prohibit using GPL code or dependencies in extensions but we highly discourage it, and none of the current extensions in the Slicer extensions index use GPL.

PythonQt is a similar (in many aspects better) Python wrapper for Qt, it is already bundled with Slicer core, and has a permissive license (LGPL) that does not limit how you use and distribute your software.

@SenonETS I would recommend to update your software to use PythonQt instead of PyQt. If you have any questions about the PythonQt syntax or how to update any specific command in your code then let us know.

@SenonETS
Copy link
Author

@lassoan I was trying to replace PyQt5 with PythonQt, however, did not find the correct way to import QFileDialog.
Could you please share any links to the PythonQt documents about using QFileDialog?
Thank you so much!

@SenonETS
Copy link
Author

SenonETS commented Oct 16, 2022

Yes Slicer uses PythonQt and not PyQt. Syntax is similar, but not exact. QFileDialog would be available in Slicer at

import qt
qt.QFileDialog

Hi James,

Yes, theoretically we can import QFileDialog from qt in 3D Slicer, however, that QFileDialog never works when I call it.
3D Slicer will freeze if I call qt.QFileDialog in the Python script. I have no idea why. And that is why I installed PyQt5.

I am not sure how to import QFileDialog from PythonQt in 3D Slicer.
Could you please share any links to the PythonQt documents about how to use QFileDialog from PythonQt?

@SenonETS
Copy link
Author

SenonETS commented Oct 16, 2022

Hi, I wanted to test the module in this extension called "sl_01__LaminaLandmark_Labeling", but I get this error message when I try to add it to my Slicer:

  File "E:/SlicerIGSpineDeformity/sl_01__LaminaLandmark_Labeling/sl_01__LaminaLandmark_Labeling.py", line 11, in <module>
    from PyQt5.QtWidgets import QMessageBox, QFileDialog
ModuleNotFoundError: No module named 'PyQt5'

Could you please add instructions to the repository README file on how to install the extension and its dependencies? If this pull request is merged, will Slicer find PyQt5? I'm not too familiar with that library, but I thought it's not part of Slicer. Thank you.

Hi Tamas,

I am not sure how to import QFileDialog from PythonQt. Would be willing to replace PyQt5 using PythonQt after I figured out.

But I did add how to install PyQt5 in my GitHub repository:

image

And you can find an example Scene File for you to test my module.

Please let me know if that works for you!

@pieper
Copy link
Member

pieper commented Oct 16, 2022

3D Slicer will freeze if I call qt.QFileDialog in the Python script. I have no idea why.

Hmm, not sure what's not working for you. Can you provide a script to replicate?

This works for me (tested on 5.0.3 macOS):

>>> d = qt.QFileDialog()
>>> d.exec()
1
>>> d.selectedFiles()
('/private/tmp/IMG_0097.jpeg',)

In addition to the license issues that @lassoan pointed out, there's also the issue that Slicer's PythonQt and pip's PyQt may have conflicting libraries that lead to crashes or other odd behavior. I have tried some PySide based code in Slicer in the past and it worked fine on some platforms but would not even load on others.

@SenonETS
Copy link
Author

Hmm, not sure what's not working for you. Can you provide a script to replicate?

Hi Steve,

In my python script in the repo, at line 1210, you can find where I called QFileDialog:
strFilePath_NumpyLandmarks, _ = QFileDialog.getOpenFileName(caption='Load target NumpyLandmark file', \ directory=slicer.mrmlScene.GetRootDirectory(), filter="NumpyLandmark file (*.npy)")

Originally, QFileDialog require an argument of QWidget as the first argument to call QFileDialog.getOpenFileName, however, in 3D Slicer ScriptedLoadableModuleWidget is not a QWidget and I was not able to give the first argument.
In this way, I have to avoid giving QWidget as the first argument while still giving the directory and filter.

This is where 3D Slicer will freeze if I call **qt**.QFileDialog.getOpenFileName instead of PyQt5.QFileDialog.getOpenFileName.
My OS is Windows10. I have no idea why qt.QFileDialog.getOpenFileName makes 3D Slicer freeze.

@pieper
Copy link
Member

pieper commented Oct 16, 2022

How about just using Slicer's main window as the parent, something like this:

qt.QFileDialog.getOpenFileName(slicer.util.mainWindow(), "my dialog", "NumpyLandmark file (*.npy)")

@SenonETS
Copy link
Author

SenonETS commented Oct 16, 2022

slicer.util.mainWindow()

I changed the code to

strFilePath_NumpyLandmarks, _ = qt.QFileDialog.getOpenFileName(slicer.util.mainWindow(),
                                                            caption='Load target NumpyLandmark file', \
                                                            directory=slicer.mrmlScene.GetRootDirectory(),
                                                            filter="NumpyLandmark file (*.npy)")

But still, 3D Slicer freezes...

@SenonETS
Copy link
Author

SenonETS commented Oct 16, 2022

Probably you could pip install PyQt, but I would not recommend it, because PyQt's free license is a restrictive viral license (GPLv3). We don't explicitly prohibit using GPL code or dependencies in extensions but we highly discourage it, and none of the current extensions in the Slicer extensions index use GPL.

PythonQt is a similar (in many aspects better) Python wrapper for Qt, it is already bundled with Slicer core, and has a permissive license (LGPL) that does not limit how you use and distribute your software.

@SenonETS I would recommend to update your software to use PythonQt instead of PyQt. If you have any questions about the PythonQt syntax or how to update any specific command in your code then let us know.

Hi @lassoan ,

I just tested PythonQt, using the code block below:

import PythonQt
strFilePath_NumpyLandmarks, _ = PythonQt.QtGui.QFileDialog.getOpenFileName( \
                                                            caption='Load target NumpyLandmark file', \
                                                            directory=slicer.mrmlScene.GetRootDirectory(),
                                                            filter="NumpyLandmark file (*.npy)")

However, just like using qt.QFileDialog, 3D Slicer freezes and I never see the dialog pops up.
I tested on two different computers: one is Windows10 Intel CPU E3-1225 v5, and the other one is Windows10 Intel CPU i9.
3D Slicer freezes on both two computers.

@pieper
Copy link
Member

pieper commented Oct 16, 2022

Strange indeed. I just tried on windows and the file dialog works for me. @SenonETS does the freeze happen if you just start a fresh slicer and past the line I posted above into the python console?

@lassoan
Copy link
Contributor

lassoan commented Oct 16, 2022

First of all, there should be no reason to open some arbitary npy files. You can store markup lists in nice, human-readable, self-descriptive json or csv file. I would recommend the json format.

Also, in general, modules should not deal file reading/writing. It should not matter what file format data nodes are loaded from and saved to. Due to this reason, we very rarely need to put file selectors in module GUIs. Instead, we use node selectors.

In the few rare cases when we select files, we usually add a ctk.ctkPathLineEdit() widget, because in general we want to avoid popup windows. This widget shows a path, so you can easily copy-paste a path, it has auto-complete, history, and has an optional button for displaying a pop-up for file selection.

If you are curious why getOpenFileName hung when you tried to use it with the code snippet above: This is a C++ call, so you cannot use named arguments to specify optional parameters as you would do it in Python functions. I don't know why it hangs (probably the expression is interpreted as a simple assignment operator, which does not have a return value), but you can ask it but if you want to find out then you can submit an issue about this to the PythonQt issue tracker. In the meantime, if you want to use this function for some reason, the correct syntax is:

strFilePath_NumpyLandmarks = qt.QFileDialog.getOpenFileName(slicer.util.mainWindow(), 'Load target NumpyLandmark file', slicer.mrmlScene.GetRootDirectory(), "NumpyLandmark file (*.npy)")

@SenonETS
Copy link
Author

SenonETS commented Oct 16, 2022

Strange indeed. I just tried on windows and the file dialog works for me. @SenonETS does the freeze happen if you just start a fresh slicer and past the line I posted above into the python console?

Problem solved!

@pieper No, your code does not freeze.

The but will freeze if I add the ArgumentName= in front of the argument.

Freeze code here:

strFilePath_NumpyLandmarks = qt.QFileDialog.getOpenFileName(slicer.util.mainWindow(),
    caption='Load target NumpyLandmark file', directory=slicer.mrmlScene.GetRootDirectory(), filter="NumpyLandmark file (*.npy)")

Not freeze code here:

strFilePath_NumpyLandmarks = qt.QFileDialog.getOpenFileName(slicer.util.mainWindow(),
            'Load target NumpyLandmark file', slicer.mrmlScene.GetRootDirectory(), "NumpyLandmark file (*.npy)")

Thank you @pieper @lassoan ! The problem is solved now!

@ungi Please help review the code :). You can download my example SceneFile from here.

@SenonETS
Copy link
Author

SenonETS commented Oct 16, 2022

Thank you @lassoan so much for your help! The problem is about assigning arguments using the argument names.
I am able to remove PyQt5 now, the problem is solved!

First of all, there should be no reason to open some arbitary npy files. You can store markup lists in nice, human-readable, self-descriptive json or csv file. I would recommend the json format.

Rearding the npy files, there are two reasons of outputing it:

  1. File save does not work very well from my side, not sure why, but you can find my question post here in which there has been no reply yet.
  2. To implement deep learning model training, Numpy file is much more convenient than the json file. I will implement auto landmark generation using the Numpy file to train the AI model. :)

@jcfr jcfr changed the title Add files via upload: Distribute the custom extension SlicerIGSpineDeformity Add SlicerIGSpineDeformity extension Aug 15, 2023
@jcfr jcfr added the Status: In Progress The review of this extension is in progress. label Aug 15, 2023
@jcfr
Copy link
Member

jcfr commented Aug 16, 2023

@SenonETS Did you have a chance to make additional progress on this ? What is the current status ?

cc: @ungi

@jcfr jcfr added Status: Awaiting Response ⏳ Waiting for a response/more information and removed Status: In Progress The review of this extension is in progress. labels Aug 16, 2023
@SenonETS
Copy link
Author

@SenonETS Did you have a chance to make additional progress on this ? What is the current status ?

cc: @ungi

Hi Jean-Christophe, thank you for asking!
This extension module has been employed in the ultrasound Lamina Landmark study published here: I shared it with my colleague, and the current version works well.

The further contribution may cover the integration of deep learning for landmark prediction, or spine deformity analysis (which may require 3D ultrasound sequences acquired from Adolescent Idiopathic Scoliosis patients). It is currently not scheduled yet.

@SenonETS
Copy link
Author

Here is an update of Publications:
IEEE TUFFC journal: Automatic 3D Lamina Curve Extraction from Freehand 3D Ultrasound Data using Sequential Localization Recurrent Convolutional Networks
SPIE Conference: Lamina landmark detection in ultrasound images: a preliminary

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Awaiting Response ⏳ Waiting for a response/more information
Development

Successfully merging this pull request may close these issues.

None yet

6 participants