tne-lab / py-behav-box-v2 Goto Github PK

Hello, first of all it's a pleasure to review your work!

So, so far I'm checking it alongside with experimentalist friend that doesn't know how to code but would like to use the library.
I would like to know who is the target of your library? Because I want to test it with this friend that will need it to plan the experiments.

In the first testing session, we did today was frustrating to him (basic python level) that you can't do everything trough the interface. We tried to follow the tutorial to add the webcam, and he got lost in creating the AdresseFile, is not clear in the tutorial how you switch to the GUI to create the file.

If you are aiming advance people in python already adapted to do this setups, I'll change my perspective.

Thanks

Describe the bug
Sources that require a string parameter fail to properly escape the quotes in the saved settings file. This leads to a crash on any subsequent startup.

To Reproduce
Add a source that has a string parameter.

Expected behavior
The string should be properly escaped in the settings file.

Approach:

Add an unavailable method to base Source class that sends a new UnavailableSourceEvent to the TaskProcess and then to the Workstation. Source subclasses can call this method under any circumstances where the Source becomes unavailable. Sources should be assumed to be available until an UnavailableSourceEvent is received.

Questions/Considerations:

Should an UnavailableSourceEvent immediately pause any running tasks that might use that Source? This would then give the user the opportunity to assess if the Task can be continued after some adjustments to the Source.

Should there be a distinction between unavailability and an unrecoverable crash? Potentially unavailable Sources could be refreshed while crashed Sources would require a full restart.

Should there be a global crash handler for each Source that handled unrecoverable errors and relays them to the main process?

Generally the NIDAQSource has some compatibility issues with multi-task behavior/repeatability

We should just remove the ability to include spaces in names

The current version does not support duplicate images

Program will spontaneously crash after long runtime with a windows fatal exception

I am going through the process of adding a source and have a couple of suggestions that may help the user.

When I first went to add a video, the program crashed.

The cause was that I was missing opencv. It would be nice if you could prevent an ignorant user like myself from crashing the program by adding a try except clause such as:

try: 
    import cv2
except:
    # show a dialog explaining that the user needs to install opencv using pip install pybehave[video] or link to the documentation

The docs do mention how to install the extra dependencies here. I suggest reiterating this information of how to add the dependency here where at the moment it only describes that opencv-python is required, not how to add it.

Another suggestion is to add labels/descriptions of the attributes or a link to documentation directly in this dialog (and other SourceParametersDialogs).

A link to included sources docs in this dialog may be useful as well:

Currently the application needs to be closed from the terminal.
Desired behavior would have closing from the GUI work correctly with Sources having their close methods called.

Removed unused functions
Add links to documentation and GitHub issues in help dropdown

Tasks need to declare the names and types of the components they use

Describe the bug
When an AddressFile is loaded with components listed out of order but in the same list, a MalformedAddressFileException is raised.

To Reproduce
Steps to reproduce the behavior:

1. Create AddressFile with list components
2. Add components out of order
3. Load AddressFile

Expected behavior
No crash, out of order adding should be allowed

Screenshots

If you are open to contributions, please mention so in the documentation as well as provide guidance on how to contribute to the repository. For example, are there particular branches that you would like pull requests to based off of (dev and docs only?), or are you okay with pull requests based on master?

A major draw of this software is that it is hardware agnostic. It would be easier to get started/evaluate/become familiar with the software if there were a tutorial that does not require Lafayette Instruments hardware and WhiskerServer as the current tutorial does.

Would it be possible to create an tutorial with an example task that only uses the pygame GUI as a source of events? Or perhaps a tutorial that connects to simulated hardware that provides events?

There are a lot of features to explore and learn about when it comes to creating task logic or extra GUI widgets etc. Right now not having the same hardware as the tutorial is a barrier.

I am trying to add a video feed and am having trouble with my AddressFile.
I created an my_address_file.py based on this page of documentation. A downloadable example address file would be helpful instead of the current code snippets provided in the docs.

my attempted address file has the following content:

from pybehave.Utilities import AddressFile
addresses = AddressFile()
addresses.add_component("video", "Video", "es", 0, None, {"fr": 30})

When trying to run a task I get the following error:

Exception in thread Thread-1 (update_gui):
Traceback (most recent call last):
  File "C:\Users\karpo\pybehave_eval\Lib\site-packages\pybehave\GUIs\GUI.py", line 60, in __init__
    file_globals = runpy.run_path(address_file, {"AddressFile": AddressFile})
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen runpy>", line 286, in run_path
  File "<frozen runpy>", line 98, in _run_module_code
  File "<frozen runpy>", line 88, in _run_code
  File "C:/Users/karpo/Desktop/py-behav/Local/my_address.py", line 2, in <module>
    addresses = AddressFile()
                ^^^^^^^^^^^^^
TypeError: 'module' object is not callable

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "C:\Users\karpo\AppData\Local\Programs\Python\Python312\Lib\threading.py", line 1073, in _bootstrap_inner
    self.run()
  File "C:\Users\karpo\AppData\Local\Programs\Python\Python312\Lib\threading.py", line 1010, in run
    self._target(*self._args, **self._kwargs)
  File "C:\Users\karpo\pybehave_eval\Lib\site-packages\pybehave\Workstation\Workstation.py", line 222, in update_gui
    self.guis[event.chamber] = gui(event, self.task_gui.subsurface(col * self.w, row * self.h, self.w, self.h), self)
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\karpo\pybehave_eval\Lib\site-packages\pybehave\GUIs\GUI.py", line 62, in __init__
    raise MalformedAddressFileError
pybehave.Utilities.Exceptions.MalformedAddressFileError

We observed components in other chambers being activated during GUI control. Unsure of the cause.

I suspect by the inclusion of a .bat file in the repository, the software is primarily intended for and/or tested on a Windows machine? If so, please mention in the documentation that it is for Windows only. If it is supposed to be a cross-platform software, mention the extent that you have tested the compatibility. I personally failed to get it installed on a Mac, but can open a separate issue for us to try and resolve if you intend on offering support. No pressure to offer cross-platform support, just wanting the dependencies to clear in the documentation (as per JOSS review criteria).

Describe the bug
The full install and hikvision install fail on Python 3.11 due to an error installing the lxml dependency:

ERROR: Failed building wheel for lxml
Running setup.py clean for lxml
Failed to build lxml
ERROR: Could not build wheels for lxml, which is required to install pyproject.toml-based projects

To Reproduce
Steps to reproduce the behavior:

1. Activate a Python 3.11 environment
2. pip install[full] or pip install[hikvision]

Expected behavior
Install should work without issue.

Temporary solution
Installing on Python 3.10 appears to work without issue

Reason for issue
The lxml wheels for Python 3.11 on Windows appears to no longer be active: https://stackoverflow.com/questions/71152710/failing-to-install-lxml-using-pip

Adding this feature will allow users to selectively install packages for the parts of pybehave they plan to use.
This feature should also include an error message indicated if necessary libraries are missing.

If a sequence is stopped midway through the task and then started again, the sequence will begin where it left off rather than starting over.