Add NavigationToolbar to Figure

[wouterpeere]

Add NavigationToolbar to Figure

[tblanke]

I have fixed the label issue.

[tblanke]

Hi @wouterpeere ,

the following points have been fixed:

• The layout of the Figure options isn't so good.
  That has been taken care of by adding the Tab Widget to the Central Widget style sheet.
• When hovering over the buttons, a rectangle appears
  The tooltip should have been shown there and is now. The Tooltip style has been added to the Central Widget style sheet.
• When doing a 'optimise load profile' exercise on the gui, the toolbar is shown twice for the first Figure.
  The Navigationtoolbar, canvas, figure and axes creation has been moved to the FigureResults class and the figure has been added as variable in the main class. So that it does not have to be created multiple times.

Can you please have a look at it and check that everthing works?

With best regards
@tblanke

[wouterpeere]

Hi @tblanke

Perfect, thanks!
I will take a look at it. If everything seems to work, I will re-open the PR.

Best,
@wouterpeere

[wouterpeere]

@tblanke I noticed you created 'fig_temperature' and 'fig_load_duration' variable in main_class.py.
Is this necessary for the NavigationToolbar or is it just a speed improvement so that whenever the result page is opened, the figure does not need to be recalculated/recreated?

In the latter case, I would prefer the figure to be saved in the DataStorage object itself instead of in the borefield object. This simplifies the code in the main class.

[tblanke]

Hi @wouterpeere ,

That is a good idea. I have made to adaptions for this and pushed in the branch.

With best regards
@tblanke

[wouterpeere]

Hi @tblanke

This already looks nicer!

I noticed some more strange behaviour:

• When you have two scenario's (lets say one with aim 1 and one with aim 2) both calculated. When you switch from one scenario to another, the figure on the result page gets bigger each time you switch...
  
  (The same happens when you keep pressing calculate when the figure is already calculated)

• [SOLVED] Secondly, when you have calculated a scenario and you change one of the parameters, the result page should be empty, which is not the case. This is not a problem in the main branch at this moment. I solved it by restating that every time the result button is pressed, display_results() should be called. This clears everything from the result page and one can only show new results if the calculation button is pressed.

• [SOLVED] Thirdly, you removed the add_link_2_show options in the gui_structure for the load duration curve. This however crashes the gui for me whenever I try to open a saved document, due to the fact that, regardless of the aim, it runs the plot_load_duration_curve function (which can only be called in specific conditions). I solved this by restating those two lines.

[wouterpeere]

I also made a change to the way the figures are saved in DS. Now, the figures were hardcoded, meaning that every time a new ResultFigure was placed in the gui_structure, this also had to be changed in the DS.
I replaced this by using the list_of_result_figures to automatically create all the attributes for the figures.

[wouterpeere]

Some more strange behaviour:

When you load a file that was calculated, the figure displays from the first time (which is perfect), however the labels are not displayed. Is it possible that they aren't saved in the DS?

[tblanke]

I can not reproduce the "figure on the result page gets bigger" issue. Can you try to update the packages to the newest version and check if that solves the issue?

[wouterpeere]

I can not reproduce the "figure on the result page gets bigger" issue. Can you try to update the packages to the newest version and check if that solves the issue?

Matplotlib and PySide6 are both the latest versions.
To replicate my issue, delete the backupfile so one can start with a 'clean version'. Just press calculate, it will display the temperature profile. If you now press calculate again, it will enlarge the figure.

[tblanke]

Does it work now?

[wouterpeere]

@tblanke, no it did not. It caused even some problems while loading.
I reversed those changes and think I solved it by removing the setattr on line 1004. This is unneeded since replace_figure() already sets the self.fig.

Can you confirm that everything is still working by you?

[tblanke]

Correct.
Yes everthing, work on my side.

[wouterpeere]

Perfect!

I will finish my review, check if all the documentation still is correct and I will do the PR.

W.r.t. testing, looking at the coverage results, about 500 lines in combine_gui_window are not tested currently. Do you think it would be possible to increase the test coverage for the gui?

[tblanke]

Hi @wouterpeere ,

Great, from my side you can start with the PR.
I will take a look at the testing.

With best regards
@tblanke

[wouterpeere]

@tblanke , is there a particular reason why you reset the location of the BACKUP.GHEtool to the default save location? I switched it to another directory because, if one overwrites this backup.ghetool document, it can cause issues.

[tblanke]

Hi,

I have changed the location, because otherwise the default location is the current working directory. This is an issue for the installed version, because then this location may be in a folder where just admins are allowed to read and write files. I think this is the problem, why the program crashed in the past.
Yes it can also cause issues if the backup file is overwritten but this can be easy fixed by just deleting it. What do you think about creating a hidden file under Documents/GHEtool?

[tblanke]

We have to create a hidden folder and no hidden file, because the hidden file is causing Permission errors. I have created a new commit for this.

[wouterpeere]

Or, instead of creating a hidden folder, we can change the extension for the backup to *.ghetoolbackup, so it won't show up in the dialog box when saving?

[tblanke]

That is a better idea. I have made the changes.

[wouterpeere]

I checked the gui and all looks fine. I've added some more Dutch translations and fixed a small bug w.r.t. the ResultTexts. I will open the PR.

@tblanke will you be able to create some more unit tests for the gui in this issue?

[tblanke]

Hi @wouterpeere ,

I will create a new branch for the gui testing.

With best regards
@tblanke