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

Added more functions to the FSLeyes plugin #2581

Merged
merged 29 commits into from
Feb 4, 2020
Merged

Conversation

jcohenadad
Copy link
Member

@jcohenadad jcohenadad commented Jan 13, 2020

The purpose of this PR is to add more SCT functions to the FSLeyes plugin. For more info, see the original PR: #2339

Useful doc for Python's wx module (used by FSLeyes): https://wxpython.org/presentations/OSCON2003/wxPython-OSCON2003-v5.pdf

Fixes #1914

This PR also fixes another issue related to the non-existence of PYTHONHOME variable in our Docker container. Fixes #2587

@lrouhier
Copy link
Contributor

lrouhier commented Jan 27, 2020

I have tested it on MACOSX. It works pretty good but there are a few problems:

  • when you want to use a function that need to use the order of the overlay to get its input (e.g., sct_register_to_template ), I get the following error:
BrokenPipeError: [Errno 32] Broken pipe
Traceback (most recent call last):
  File "/Users/lurou_admin/Desktop/sct/sct/contrib/fsl_integration/sct_plugin.py", line 540, in onButtonCSA
    img = overlayORD[0]
NameError: name 'overlayORD' is not defined
  • The text and the logo does not appear before I try to scroll down the text or click on the logo. (see screenshot below)

before scrolling text down
Screen Shot 2020-01-27 at 4 15 13 PM

After scrolling text down
Screen Shot 2020-01-27 at 4 15 24 PM

@jcohenadad
Copy link
Member Author

jcohenadad commented Jan 27, 2020

@lrouhier thanks for the feedback

when you want to use a function that need to use the order of the overlay to get its input (e.g., sct_register_to_template ), I get the following error:

i'm still working on a major refactoring-- in the future, the overlay order will not be used. We will follow the approach already implemented in propseg, deepseg_sc and deepseg_gm

The text and the logo does not appear before I try to scroll down the text or click on the logo. (see screenshot below)

hum... do you also see that in propseg panel? what OS are you using? does the terminal show any relevant error/warning?

@lrouhier
Copy link
Contributor

i'm still working on a major refactoring-- in the future, the overlay order will not be used. We will follow the approach already implemented in propseg, deepseg_sc, and deepseg_gm

Perfect because these three worked really well!

hum... do you also see that in propseg panel? what OS are you using? does the terminal show any relevant error/warning?

I am on MAcOSX Catalina with Fsleyes 0.27.1. There are no errors in the terminal. However, the display is working perfectly fine for sct_propseg...

@jcohenadad
Copy link
Member Author

However, the display is working perfectly fine for sct_propseg...

good to know. i will continue the refactoring and let you know when ready to be fully tested

@jcohenadad jcohenadad added enhancement category: improves performance/results of an existing feature sct_plugin.py (FSLeyes) context: Related to FSLeyes plugin labels Jan 28, 2020
@jcohenadad jcohenadad added this to the 4.2.2 milestone Jan 28, 2020
@lrouhier
Copy link
Contributor

I tested it outside of docker everything work except the citation, explanation and logo display (same issue as yesterday on the aesthetic). It does fix the fact that the plugin was not responsive in docker, However there still all the warning from opengl reported in #2587. I think the warning should be a separate issue though, it seems to come from a problem between fsleyes and Pyopengl.

@jcohenadad
Copy link
Member Author

I think the warning should be a separate issue though, it seems to come from a problem between fsleyes and Pyopengl.

Agreed-- this is a docker-specific issue.

@jcohenadad
Copy link
Member Author

jcohenadad commented Jan 30, 2020

I tested it outside of docker everything work except the citation, explanation and logo display

where/how did you test it? with what version of FSLeyes and which commit?

@lrouhier
Copy link
Contributor

lrouhier commented Jan 31, 2020

where/how did you test it? with what version of FSLeyes and which commit?

version: git-jca/1914-fsleyes-b228ab9160a7f27a55c0c6f6822dd5c22de1964d
MacosX Catalina
fsleyes/FSLeyes version 0.30.0

I tested it by opening fsleyes (fsleyes &) and then use the run script option inside fsleyes. I then ran all the function but sct_deepseg_gm on the sct_example_data/t2/t2.nii.gz images. (i use the segmentation I got with sct_deepseg_sc as input afterward as well.

@jcohenadad
Copy link
Member Author

I tested the plugin (commit ) using FSLeyes 0.31.0 (package install) as well as FSLeyes 0.30.0 (conda install), under OSX Mojave, and did not observe anything wrong with the logo and text. So, @lrouhier i will follow up with you internally to test it directly on your station.

@jcohenadad
Copy link
Member Author

@lrouhier the display issue you are experiencing is outside the scope of this PR, and i opened an issue here #2595.
if you have no other comment can you pls approve (or further comment) this PR?

Copy link
Contributor

@lrouhier lrouhier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. All other tests worked for me.

@jcohenadad jcohenadad merged commit ca93f23 into master Feb 4, 2020
@jcohenadad jcohenadad deleted the jca/1914-fsleyes branch February 4, 2020 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement category: improves performance/results of an existing feature sct_plugin.py (FSLeyes) context: Related to FSLeyes plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unresponsive command in FSLeyes integration within Docker Add more functions to fsleyes integration
2 participants