twVolc / PyCamPermanent

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

Issue 116/display and save config #128

Closed ubdbra001 closed 1 month ago

ubdbra001 commented 1 month ago

Fixes #116

This adds a frame to display the currently specified spec_dir for DOAS It should also ensure that the updated spec_dir (either thought the GUI or a config) is saved to a new config properly.

Not sure if it resolves the previously discussed reset of the Dark Spectra/ Spectra directories, so give it a try and if it doesn't let me know.

twVolc commented 1 month ago

I'm wondering if DirIndicator should go in doas_analysis.py rather than misc.py? This would just keep all of those widgets on that side of the pane in the same place, so probably makes it easier to find if needed.

Also, possibly should be implemented separately to this PR, but just wondering whether we need to bear in mind how things might work if they were to be run outside of a GUI. In this case, the call to self.dir_info.update_dir() would raise an error as presumably the dir_info attribute wouldn't have been assigned a DirIndicator object. Just with that potential future in mind I wonder if it's worth putting a check in before calling update_dir() - e.g.:

if isinstance(self.dir_info, DirIndicator):
    self.dir_info.update_dir()

or a more generic one that doesn't requrie aany knowledge of the DirIndicator class's existence (whether this is important I don't know):

if self.dir_info is not None:
    self.dir_info.update_dir()

Do you think it's worth including either here?

ubdbra001 commented 1 month ago

I'm wondering if DirIndicator should go in doas_analysis.py rather than misc.py? This would just keep all of those widgets on that side of the pane in the same place, so probably makes it easier to find if needed.

Happy to move it. I only chose misc.py as I wasn't sure where it should go, so if you think it makes more sense in doas_analysis.py then there I'm happy to follow your lead.

wondering whether we need to bear in mind how things might work if they were to be run outside of a GUI. In this case, the call to self.dir_info.update_dir() would raise an error as presumably the dir_info attribute wouldn't have been assigned a DirIndicator object.

Yes, this is an excellent point. I'll add a check to skip this if the attribute is None.

twVolc commented 1 month ago

Great. Yep let's go with doas_analysis.py, I think if I came back to things in a year or so that would be the first place I would look for the code for that part of the GUI. Maybe right at the top of that file as it's a small class (it will get lost down the bottom). Just worth double checking it all actually runs once you've moved its location - I have found a lot of issues with circular imports in the past, where I've ended up having to put classes in different files because otherwise multiple files end up wanting to import each other and it all just breaks...

ubdbra001 commented 1 month ago

Do you mean figures_doas.py? Can't find doas_analysis.py

twVolc commented 1 month ago

Do you mean figures_doas.py? Can't find doas_analysis.py

Sorry yes haha