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

Standalone app's File > Open action fails if a file was already opened #122

Closed
jpgill86 opened this issue Dec 23, 2020 · 11 comments · Fixed by #123
Closed

Standalone app's File > Open action fails if a file was already opened #122

jpgill86 opened this issue Dec 23, 2020 · 11 comments · Fixed by #123
Labels
Milestone

Comments

@jpgill86
Copy link
Collaborator

jpgill86 commented Dec 23, 2020

Thanks to @munrokrulle for reporting.

When the standalone app is first launched, StandAloneViewer (a subclass of MainViewer) presents the user with a File > Open menu action. This menu action remains available after a file has been selected and the StandAloneViewer has been populated with viewers. Using the menu action a second time to load a second file fails with this error: AssertionError: Viewer already in MainViewer. This occurs because compose_mainviewer_from_sources() is trying to populate the original StandAloneViewer a second time with viewers that already exist.

I can imagine 3 alternatives for handling this:

  1. Remove the menu action after the first file is loaded, so that the option to load a second file is not available. To view another file, users would have to launch another instance of the app (e.g., from a second console window, or by first closing the original instance, or by running each instance as a background process).
  2. Remove all viewers from the original StandAloneViewer before repopulating it with viewers showing the new file contents, so that only one file can be viewed at a time. I think that's what this "TODO" was suggesting.
  3. Spawn a new StandAloneViewer each time a file is opened, so that multiple files can be open in different windows.

My preference would be option 3 since it's the most versatile for users. I don't think this will be more difficult to implement than what I have already done for neurotic (see this, this, this, and this). @samuelgarcia, do you have a preference?

@munrokrulle
Copy link

Thanks for addressing this! I'm planning on using ephyviewer for an undergraduate class, where students will have to access multiple files. So I think options 2 or 3 would work best.

@jpgill86
Copy link
Collaborator Author

@munrokrulle: I've implemented option 3 in #123. Give it a try and tell me if it works for you. You can install that version using this command: pip install -U git+https://github.com/jpgill86/ephyviewer@standalone-multi-windows

@jpgill86 jpgill86 added this to the 1.4.0 milestone Dec 25, 2020
@munrokrulle
Copy link

It works! Thank you so much!

@jpgill86
Copy link
Collaborator Author

jpgill86 commented Jan 2, 2021

You're welcome!

I hope the class goes well! I'll be interested in hearing what your students think about ephyviewer!

@jpgill86
Copy link
Collaborator Author

jpgill86 commented Jan 4, 2021

The solution in #123 has been merged into master, so this should now be sufficient to install it:

pip install -U git+https://github.com/NeuralEnsemble/ephyviewer

I hope to create a release in the near future which will include this fix and other recent changes. @munrokrulle, when does your class start?

@munrokrulle
Copy link

Classes start on Jan. 25, and we should be introducing ephyviewer the week after that. I'm going to have them install it with Anaconda, which has worked for me. They could then type in the above command after the environment is set up.

@jpgill86
Copy link
Collaborator Author

jpgill86 commented Jan 4, 2021

Great, I can make sure the new version is available by then. 👍

@jpgill86
Copy link
Collaborator Author

Hi @munrokrulle, I released ephyviewer 1.4.0 today, which includes the fix for your issue: https://ephyviewer.readthedocs.io/en/latest/releases/1.4.0.html

You can install it with: pip install -U ephyviewer

@jpgill86
Copy link
Collaborator Author

You can install it with: pip install -U ephyviewer

Since your students will be using Anaconda, I should have mentioned that you can also install it from the conda-forge channel:

conda install -c conda-forge ephyviewer

@munrokrulle
Copy link

Great! That's how I installed it previously. I just checked, and the same installation works for the new version.

I install it by setting up a new environment in Anaconda (with Python 3.8). Then I add the packages: pyqt, ephyviewer, python-neo, and av. Then I can run the stand-alone version in a terminal (simplest for students), or on the Qt Console.

@jpgill86
Copy link
Collaborator Author

Perfect, that's exactly what I'd recommend!

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

Successfully merging a pull request may close this issue.

2 participants