volcanotech-sw / PyCamPermanent

Permanent PiCam (SO2) installation project software
GNU General Public License v3.0
2 stars 3 forks source link

Silent return when PCS Line not found #239

Open christopher-wild opened 1 month ago

christopher-wild commented 1 month ago

I've been trying to load a config file provided by Dan with the GUI and on the GUI it appeared that nothing was happening. Looking at the stack trace I was recieving the following exception:

Exception in Tkinter callback
Traceback (most recent call last):
  File "c:\Users\cs1xcw\Documents\Projects\PyCamPermanent\.conda\lib\tkinter\__init__.py", line 1892, in __call__
    return self.func(*args)
  File "C:\Users/cs1xcw/Documents/Projects/PyCamPermanent\pycam\gui\menu.py", line 730, in load_config_file
    self.reload_config()
  File "C:\Users/cs1xcw/Documents/Projects/PyCamPermanent\pycam\gui\menu.py", line 737, in reload_config
    self.reload_all()
  File "C:\Users/cs1xcw/Documents/Projects/PyCamPermanent\pycam\gui\menu.py", line 714, in reload_all
    self.load_all()
  File "C:\Users/cs1xcw/Documents/Projects/PyCamPermanent\pycam\gui\menu.py", line 704, in load_all
    self.set_all_pcs_lines()
  File "C:\Users/cs1xcw/Documents/Projects/PyCamPermanent\pycam\gui\menu.py", line 592, in set_all_pcs_lines
    self.load_pcs(filename=line, new_line=True)
  File "C:\Users/cs1xcw/Documents/Projects/PyCamPermanent\pycam\gui\menu.py", line 565, in load_pcs
    self.pyplis_worker.fig_tau.add_pcs_line(line, line_num=line_num, force_add=True)
  File "C:\Users/cs1xcw/Documents/Projects/PyCamPermanent\pycam\gui\figures_analysis.py", line 695, in add_pcs_line
    line.line_id = lbl
AttributeError: 'NoneType' object has no attribute 'line_id'

After some debugging I found that line_id was a none type because when pcs_lines are loaded if the file is not found there is a silent return. I think we should change this behaviour to raise an exception which details the file wasn't found and feeds this back to the GUI. This will help future debugging as the stack trace will reflect the issue better.

https://github.com/twVolc/PyCamPermanent/blob/ccd5085f57ef4a63b52af450f4be25bd62d89a10/pycam/io_py.py#L183-L191

twVolc commented 1 month ago

I've seen this recently too actually. I can't remember how I fixed it - I think I changed the config file PCS line name or something like this. Not sure why I didn't post it as an issue.

Is this working on the dev branch? Some recent changes to that might be set up to catch this error, although I have a feeling I still hit this on dev recently.

christopher-wild commented 1 month ago

Just checked dev (i was on main by mistake!) but it still happens on dev unfortunately. I'll continue digging to see if I can find why the file can't be found

twVolc commented 1 month ago

For context, it was relatively recently that we did a big update to config files, to make the file paths a little more robust to moving config folders around (i.e. all files in the config folder are now just filenames where previously they were all written as absolute paths). See #151 and #221 for details. As this is quite new it's possible there's something buggy in there. Or perhaps this is totally unrelated...

christopher-wild commented 1 month ago

Thank you, they were helpful!! It was to do with relative paths, I'd moved where the config was relative to the rest of the files.

I'll leave this issue though as the silent error passing can be considered bad practise

twVolc commented 1 month ago

Absolutely, I'm surprised this hasn't raised warning box in the GUI as that was what #225 addressed and we thought we'd caught most cases there. This must be a case where that is missed so I think definitely worth keeping this open. @ubdbra001 can probably fix this easily as he knows his way around the config loading!

ubdbra001 commented 1 month ago

I'd expect this to happen in the main branch, but not on dev because of #225. I'm surprised that it didn't catch that the file was missing when you first load the config.

@christopher-wild could you give me an idea of what the directory/file structure looked like when you ran into this error?

ubdbra001 commented 1 day ago

Should replace the print statement with writing to log e.g. (logging.warning)