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

Issues to be fixed before submitting the extension #3

Closed
6 tasks done
lassoan opened this issue Feb 2, 2020 · 18 comments
Closed
6 tasks done

Issues to be fixed before submitting the extension #3

lassoan opened this issue Feb 2, 2020 · 18 comments

Comments

@lassoan
Copy link
Contributor

lassoan commented Feb 2, 2020

@ReynoldsJ20
Copy link
Owner

Hi Andras, did you see my question in the thread which you linked above? Thanks.

@lassoan
Copy link
Contributor Author

lassoan commented Feb 4, 2020

I've answered now.

@ReynoldsJ20
Copy link
Owner

ReynoldsJ20 commented Feb 7, 2020

Hi, I think I finally have got the parameter node to do what I want it to do i.e. when I save and open the scene the FidcualsToModelDistance has all the same settings and inputs as when I saved the scene. I can also make multiple parameter nodes and change their settings and the settings are saved in the parameter node are restored when I select that parameter node again. I found this very complicated and I still don't fully understand how or why it works so there may be some strange/redundant code in there. I borrowed a lot of the parameter node code from VectorToScalarVolume as this had a non-singleton parameter node setup. The updated code is in the parameter node branch

@lassoan
Copy link
Contributor Author

lassoan commented Feb 7, 2020

Thank you. I've sent a pull request (#4) with an overall cleanup and simplification of the module.

@ReynoldsJ20
Copy link
Owner

Thanks so much, that looks a lot more clean. Hopefully it didn't take you too long and my coding attempts weren't too much of a hindrance.

I was happy to see these lines from ``updateGUIFromParameterNode`

wasBlocked = self.ui.inputPointsSelector.blockSignals(True)
self.ui.inputPointsSelector.setCurrentNode(self._parameterNode.GetNodeReference("InputPoints"))
self.ui.inputPointsSelector.blockSignals(wasBlocked)

I spent a long time yesterday trying to figure out how to stop the parameters updating again when the ``updateGUIFromParameterNode` changed the nodes in the input selector boxes. I ended up making separate methods for each node which worked but it didn't seem right.

What would be the difference between the code above and this:

self.ui.inputPointsSelector.blockSignals(True)
self.ui.inputPointsSelector.setCurrentNode(self._parameterNode.GetNodeReference("InputPoints"))
self.ui.inputPointsSelector.blockSignals(False)

I was just wondering why you structured it with the wasBlocked variable?

Thanks again.

@lassoan
Copy link
Contributor Author

lassoan commented Feb 8, 2020

After we temporarily block signals we must restore the previous blocked/unblocked state. We cannot just simply unblock it.

@ReynoldsJ20
Copy link
Owner

Great thanks, I had a play in the python inter-actor and I see how it restores the original state with the true/false outputs.

What do I need to do now to submit the extension?

@lassoan
Copy link
Contributor Author

lassoan commented Feb 8, 2020

You just need to update these lines: https://github.com/ReynoldsJ20/SlicerFiducialsToModelDistance/blob/master/CMakeLists.txt#L7-L13

Also update description of the extension (see link above) and module (https://github.com/ReynoldsJ20/SlicerFiducialsToModelDistance/blob/master/FiducialToModelDistance/FiducialToModelDistance.py#L22) to reflect that it can compute error for two sets of fiducials, too (not just between mode and fiducials).

@ReynoldsJ20
Copy link
Owner

Ok, I think I have done both those things correctly.

@lassoan
Copy link
Contributor Author

lassoan commented Feb 8, 2020

Icon urls must be the download urls, for example https://raw.githubusercontent.com/ReynoldsJ20/SlicerFiducialsToModelDistance/master/FiducialtoModelDistance.png

Nitpick in extension description: results are displayed in two tables (not just "a table")

@ReynoldsJ20
Copy link
Owner

Both descriptions updated and icon URL is updated.

@lassoan
Copy link
Contributor Author

lassoan commented Feb 8, 2020

@ReynoldsJ20
Copy link
Owner

Ok, I have put the two download links with spaces in between

@lassoan
Copy link
Contributor Author

lassoan commented Feb 8, 2020

Thank you, I've submitted the extension to the extension index. If everything goes well then it will show up in Slicer's Extension Manager tomorrow.

@ReynoldsJ20
Copy link
Owner

Great thanks for the help!

@lassoan
Copy link
Contributor Author

lassoan commented Feb 8, 2020

The module shows up and can be installed form the extension manager but it does not appear in the module list. The problem was that the module name was spelled inconsistently (to/To and Fiducial/Fiducials). I've sent a pull request (#5) that should fix it.

Please also add the "3d-slicer-extension" topic to the repository:

  • Repository is associated with 3d-slicer-extension GitHub topic so that it is listed here. To edit topics, click "Manage topics" on your repository's main page, on the left, near the top (just below the repository description). To learn more about topics, read https://help.github.com/en/articles/about-topics

@ReynoldsJ20
Copy link
Owner

Thanks for the pull request. I have added the 3d slicer topic as mentioned above. Thanks again, I think I am slowly learning slicer programming with your help.

@ReynoldsJ20
Copy link
Owner

Thanks, pull request is merged now

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