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.

Sign up for GitHub

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

Jump to bottom

GUI #74

Merged
merged 18 commits into from
Jul 8, 2024
Merged

GUI #74

merged 18 commits into from
Jul 8, 2024

Conversation

rnmitchell
Copy link
Contributor

Creating a GUI using streamlit. Code written by Shaheer Syed.

@standage
Copy link
Member

I'm getting the following error, whether I run lusstr gui or streamlit run lusSTR/cli/gui.py.

$ lusstr gui

  You can now view your Streamlit app in your browser.

  Local URL: http://localhost:8501
  Network URL: http://192.168.101.24:8501

*** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'NSWindow should only be instantiated on the main thread!'
*** First throw call stack:
(
        0   CoreFoundation                      0x00007ff804a31dc6 __exceptionPreprocess + 242
        1   libobjc.A.dylib                     0x00007ff804521e9d objc_exception_throw + 48
        2   CoreFoundation                      0x00007ff804a5578c -[NSException raise] + 9
        3   AppKit                              0x00007ff808041b01 -[NSWindow _initContent:styleMask:backing:defer:contentView:] + 1595
        4   AppKit                              0x00007ff8080414c0 -[NSWindow initWithContentRect:styleMask:backing:defer:] + 42
        5   libtk8.6.dylib                      0x000000011a288274 TkMacOSXMakeRealWindowExist + 740
        6   libtk8.6.dylib                      0x000000011a287dc2 TkWmMapWindow + 82
        7   libtk8.6.dylib                      0x000000011a1d7a21 MapFrame + 65
        8   libtcl8.6.dylib                     0x000000011a41e647 TclServiceIdle + 135
        9   libtcl8.6.dylib                     0x000000011a3fc2b1 Tcl_DoOneEvent + 385
        10  libtk8.6.dylib                      0x000000011a27775e TkpInit + 766
        11  libtk8.6.dylib                      0x000000011a1cf8ba Initialize + 2602
        12  _tkinter.cpython-311-darwin.so      0x0000000110e10738 Tcl_AppInit + 88
        13  _tkinter.cpython-311-darwin.so      0x0000000110e1044a Tkapp_New + 586
        14  _tkinter.cpython-311-darwin.so      0x0000000110e101ee _tkinter_create_impl + 222
        15  _tkinter.cpython-311-darwin.so      0x0000000110e0fe1b _tkinter_create + 187
        16  python3.11                          0x000000010f968143 cfunction_vectorcall_FASTCALL + 99
        17  python3.11                          0x000000010fa5ba44 _PyEval_EvalFrameDefault + 231956
        18  python3.11                          0x000000010f904898 _PyFunction_Vectorcall + 3064
        19  python3.11                          0x000000010f9065ca _PyObject_Call_Prepend + 186
        20  python3.11                          0x000000010f9986e9 slot_tp_init + 185
        21  python3.11                          0x000000010f98d6da type_call + 122
        22  python3.11                          0x000000010fa5c683 _PyEval_EvalFrameDefault + 235091
        23  python3.11                          0x000000010fa210d4 _PyEval_Vector + 2996
        24  python3.11                          0x000000010fa204a9 PyEval_EvalCode + 249
        25  python3.11                          0x000000010fa1b3ad builtin_exec + 381
        26  python3.11                          0x000000010f9680ad cfunction_vectorcall_FASTCALL_KEYWORDS + 93
        27  python3.11                          0x000000010fa5ba44 _PyEval_EvalFrameDefault + 231956
        28  python3.11                          0x000000010f904898 _PyFunction_Vectorcall + 3064
        29  python3.11                          0x000000010f909baa method_vectorcall + 458
        30  python3.11                          0x000000010fa62162 _PyEval_EvalFrameDefault + 258354
        31  python3.11                          0x000000010f904898 _PyFunction_Vectorcall + 3064
        32  python3.11                          0x000000010f909baa method_vectorcall + 458
        33  python3.11                          0x000000010fb57869 thread_run + 249
        34  python3.11                          0x000000010fae0704 pythread_wrapper + 36
        35  libsystem_pthread.dylib             0x00007ff8048e218b _pthread_start + 99
        36  libsystem_pthread.dylib             0x00007ff8048ddae3 thread_start + 15
)
libc++abi: terminating due to uncaught exception of type NSException

@rnmitchell
Copy link
Contributor Author

That's the error I got too.

@standage
Copy link
Member

Just pushed some changes to fix the issue, inspired by this thread. But now it looks like the menu needs troubleshooting.

@rnmitchell
Copy link
Contributor Author

Yeah, now I'm getting that error when I open the file picker.

@rnmitchell
Copy link
Contributor Author

I found this thread that may work, but I'm nervous to do any sudo code. 😬

@standage
Copy link
Member

I was able to get the menu working by downgrading streamlit to version 1.32. Looks like this is a known issue with streamlit_option_menu: see victoryhb/streamlit-option-menu#70.

Now I've reproduced the file picker bug. Looking into it...

@rnmitchell
Copy link
Contributor Author

@standage whenever you get a chance, please test the file/folder pickers to see if they are now working for you.

@standage
Copy link
Member

standage commented Jul 2, 2024

@standage whenever you get a chance, please test the file/folder pickers to see if they are now working for you.

Just tested on some example data. Both the file picker and the entire workflow functioned as expected!

@rnmitchell
Copy link
Contributor Author

I think this may be a good spot for merging. I plan to do some more work on the GUI, but I think it's a little bit more involved and wanted to get the basic version pushed to the master branch. The only issue I'm currently running into is the failing CI (which seems to be another tk issue??). Not too sure how to circumvent those errors.

@rnmitchell rnmitchell marked this pull request as ready for review July 3, 2024 16:35
@rnmitchell rnmitchell requested a review from standage July 3, 2024 16:36
standage
standage approved these changes Jul 8, 2024
View reviewed changes
Copy link
Member

@standage standage 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, works well on my end. As we discussed, we'll want to clean up the code as well as update the visual aesthetics of the app. But happy to merge for now.

lusSTR/cli/gui.py
Comment on lines +79 to +90
def file_picker_dialog():
script_path = importlib.resources.files("lusSTR") / "scripts" / "file_selector.py"
result = subprocess.run(["python", script_path], capture_output=True, text=True)
if result.returncode == 0:
file_data = json.loads(result.stdout)
file_path = file_data.get("file_path")
if file_path:
return file_path
else:
st.error("No folder selected")
else:
st.error("Error selecting folder")
Copy link
Member

Choose a reason for hiding this comment

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

Clever trick!

lusSTR/cli/gui.py Outdated
Comment on lines 27 to 37
import tkinter as tk
from tkinter import filedialog



# Create a global Tkinter root window
try:
root = tk.Tk()
root.withdraw() # Hide the root window
except:
print("No GUI available!")
Copy link
Member

Choose a reason for hiding this comment

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

We can drop this code now, right?

Copy link
Contributor Author

@rnmitchell rnmitchell Jul 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This is what is allowing the GitHub CI to pass. Without it, it fails.

Copy link
Member

Choose a reason for hiding this comment

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

That's really surprising to me. The file_selector and folder_selector scripts that actually use tk don't touch this code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seemed to be hung up on the from lusSTR.cli import gui line from the cli/__init.py__ script.

Copy link
Member

Choose a reason for hiding this comment

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

I understand that having root = tk.Tk() in there without a try/except block will cause issues on CI. What I'm suggesting is that we shouldn't need tk anywhere in gui.py now, since that's all been moved to accessory scripts where tk can be run in the main thread.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oooooooooh. Yes you are right!

@rnmitchell
Copy link
Contributor Author

I think this is ready now.

@standage standage merged commit 3c2b19b into master Jul 8, 2024
2 checks passed
@standage standage deleted the gui branch July 8, 2024 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@standage standage standage approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rnmitchell @standage