Plotting an already plotted dataset causes the new plot to only show model

[m2cci-NMZ]

When a dataset is already plotted, plotting the same dataset in a different Fitpage only shows the model in the plot.

[krzywon]

This seems very similar to #1454. Was it not fully fixed?

[m2cci-NMZ]

Indeed, @rozyczko fixed it in PR #1481, but somehow the commit didn't make it to ESS_GUI branch? This is strange as it shows as merged.

[krzywon]

Looking at the git history for DataExplorer.py, the change made in PR #1481 was removed to fix an issue with 2D plots in PR #1508.

[butlerpd]

wow - nice catch @krzywon!!.. but I am really getting confused!  How is this possible? Moreover, this kind of thing seems to be happening not that infrequently any more (a new PR overwriting an older one)  I thought the whole point of version control software was to prevent this kind of thing?  How come the PR merged the old code back in and overwriting the new? Clearly there is something about version control I don't understand here. Most importantly, what can we do to avoid this continuing to happen in the future?

[krzywon]

@butlerpd I think the code added in #1481 to force a single 1D plot caused a different issue with 2D plots that was addressed in #1508. This was likely more an issue with how the original PR was tested and not with version control.

[rozyczko]

Indeed this seems to have been an issue with the unit testing (or lack of thereof) for this bug.
One thing to remember is to always write unit tests, especially for bugs that got fixed, this would allow us to see failures immediately.
We used to be pretty stringent wrt unit tests for all functionality but got slightly less so the less time we had, especially before releases.
I think we should insist (as we originally did) on having unit tests for everything new that goes into the code. Part of the code review is also making sure that there is a (working) unit test committed.

[rozyczko]

Fixed the logic and added tests. Hopefully this sorts things out for all the edge cases.

[krzywon]

Was this resolved when the PR was merged? We should close the ticket if that is the case.