[BUG] An infinite loop and a few crashes

vyrjana commented 5 months ago

First of all, thanks for the continued development of pyDRTtools!

I noticed a few bugs while testing out the latest version (commit 3694b9b).

Infinite loop after clicking the "Hilbert Transform" button.

This is the relevant loop: https://github.com/ciuccislab/pyDRTtools/blob/3694b9b4cef9b29d623bef7300280810ec351d46/pyDRTtools/runs.py#L422-L432 The reason is that theta_0 is a numpy.ndarray with more than one dimension, which causes scipy.optimize.minimize to raise a ValueError ('x0' must only have one dimension.) here: https://github.com/ciuccislab/pyDRTtools/blob/3694b9b4cef9b29d623bef7300280810ec351d46/pyDRTtools/BHT.py#L151

Aside from making sure that theta_0 is 1D, you might also wish to narrow down the type(s) of exception(s) that are allowed to keep the loop running instead of using a bare except.

Crashes when closing a save file dialog by canceling

The functions related to the "DRT" and "EIS Regression" export buttons do not properly handle the case where the file dialog is closed by canceling:

If the value assigned to path is an empty string, then a FileNotFoundError will be raised later in the function when trying to write to the path. These crashes only occur if one of the three types of runs (simple, Bayesian, or HT) have been performed due to the if-elif blocks.

The QFileDialog.getSaveFileName returns an empty string if the modal window is closed by canceling, but it can also return an invalid path if the user types in a path (e.g. /tmp/foo/bar on Linux) into the File name input field in the modal window. So, I would recommend something along the lines of:

path, ext = QFileDialog.getSaveFileName(...)
if path == "":
    # The user closed the modal window by canceling.
    return
elif not os.path.isdir(os.path.dirname(path)):
    return
elif os.path.isdir(path):
    return

There might be some edge case that I'm missing here. The user is likely to assume that the file was saved unless they are somehow made aware when the path is invalid. Qt5 has a QErrorMessage class that could be used.

The following instance, which is associated with saving a figure, does not cause a crash, but it does cause a few lines about an empty or null file name to be printed to the terminal: https://github.com/ciuccislab/pyDRTtools/blob/3694b9b4cef9b29d623bef7300280810ec351d46/pyDRTtools/GUI.py#L304

Crash when exporting "EIS Regression" results

This crash occurs when experimental data has been loaded but none of the three types of runs have been performed. The reason for this is that the following if-else block assumes that at least a simple or Bayesian run has been performed.

https://github.com/ciuccislab/pyDRTtools/blob/3694b9b4cef9b29d623bef7300280810ec351d46/pyDRTtools/GUI.py#L263-L295

ciuccislab commented 4 months ago

Hello,

Thank you for your interest in pyDRTtools. Instead of using the version from PYPI, could you please git-clone the repository directly and launch the interface accordingly, as the current version will be updated on PYPI later. We are still testing it. I just launched the version from GitHub, and everything works fine. Please let me know if you encounter any other issues."

ciuccislab commented 4 months ago

Crash when exporting "EIS Regression" results

Thank you for testing and calling our attention to this. This will be fixed and updated in the current version before releasing it.

Thank you

ciuccislab commented 4 months ago

Crash when exporting "EIS Regression" results

Could you please provide the data where you encountered this issue so we can test it? I've tested with some data on my end, and everything still works fine. Your assistance in providing the specific data would be greatly appreciated. I look forward to your response. Thank you in advance.

vyrjana commented 4 months ago

I did not install pyDRTtools via PyPI. I cloned this repository (see the first message for the specific commit), created and activated a new virtual environment, installed pyDRTtools as an editable package that points to the cloned repository, and installed all of the required dependencies.

Regarding the last issue (exporting "EIS regression" results); this is very much an edge case that is unlikely to affect most people if they are using the program normally. I only noticed the issue when I started looking for the causes for the other issues I reported.

If you start the program, import some data, press the button for exporting the "EIS Regression" results, and provide a valid file path, then the program should crash when it reaches the following line: https://github.com/ciuccislab/pyDRTtools/blob/3694b9b4cef9b29d623bef7300280810ec351d46/pyDRTtools/GUI.py#L293

Following these steps, I get the following exception: AttributeError: 'EIS_object' object has no attribute 'mu_Z_re'. If you performed any of the runs (Simple, Bayesian Run, or Hilbert Transform) after importing some data, then the attributes accessed on that line should exist and an exception is not raised.

ciuccislab / pyDRTtools