Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FitResult.plot() cannot be modified after plotting #85

Closed
luisfabib opened this issue Mar 6, 2021 · 4 comments · Fixed by #87
Closed

FitResult.plot() cannot be modified after plotting #85

luisfabib opened this issue Mar 6, 2021 · 4 comments · Fixed by #87
Labels
bug Something isn't working
Milestone

Comments

@luisfabib
Copy link
Collaborator

luisfabib commented Mar 6, 2021

A small change is applied to fitsignal.py: Remove/Comment line 611:
python # plt.show()
This is important so that axs can be manipulated afterwards. I would recommend to skip the plt.show() command anyway, or manipulation of the returned axis isn't possible (at least for me). In my IDE (Spyder) plots are shown after running a script/command by default.

Originally posted by @laenan8466 in #74 (comment)

@luisfabib luisfabib added the bug Something isn't working label Mar 6, 2021
@luisfabib
Copy link
Collaborator Author

The plt.show() command is needed for some IDE's (e.g. VSCode). Actually, while not very clearly documented, the method FitResult.show() returns the list of matplotlib.AxesSubplot objects.

The issue is that while modifiable, the matplotlib.AxesSubplot are (at least for my Matplotlib knowledge) tricky or difficult to replot. I will change the output to return the matplotlib.figure object. That all elements inside the figure can be accessed, modified, and replotted by a simple display(fig).

A PR will follow soon.

@laenan8466
Copy link
Contributor

laenan8466 commented Mar 6, 2021

Thank you in advance for a PR. :-)
Just to add, there are more plt.show() calls in the DeerLab code:

  • fitparamodel.py line 281
  • deerload.py line 226
  • fitregmodel.py line 401
  • fitmultimodel.py line 484
  • snlls.py line 480
  • fitsignal.py line 580 (was discussed above)

Haven't looked if all these calls result from a FitResult.plot() call. Sorry if this was already clear to you.

As mentioned, it would be neat to have output of both, matplotlib.AxesSubplot and matplotlib.figure.

Wouldn't it be more flexible to use fix, axs = FitResult.plot() to obtain the axis and figure (without plot) and in the case that a plot command is needed use the FitResult.show() routine to plot everything as needed (or just call plt.show())?

@luisfabib
Copy link
Collaborator Author

luisfabib commented Mar 7, 2021

Yes I am aware. The plt.show() calls are for the FitResult.plot() method (except for deerload).

Instead of the syntax fig, axs = FitResult.plot() I would be more inclined towards just returning the matplotlib.figure object via fig= FitResult.plot(), since the axes objects are just children of that one: ax_i = fig.axes[i].

I am also not against but not sure about a separation of FitResult.plot() and FitResult.show(). From a user perspective, it is not probable that the average user will dwell too much into the OOP-side of Matplotlib. And having two methods called plot() and show() might cause confusion regarding its functionality.
I see that when we want to manipulate the plot objects then we do not want FitResult.plot() to be rendering the figure yet. My idea was to include a keyword fig = FitResult.plot(show=True/False) (also found in some Matplotlib objects) that would enable/disable the rendering of the plot upon calling but still returning the figure object. This way, the number of outputs is still concise and advanced users can still manipulate the functionality freely.

@laenan8466
Copy link
Contributor

Your solution sounds to be a far superior method! :-) Works for me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants