Use of matplotlib's OOP API

[mocquin]

Most use of matplotlib througout the package rely on the implicit pyplot API.

I'd advocate for using the more explicit OOP-oriented API since it is considered cleaner, more pythonic, and allows for more control over plots.

The difference between the 2 APIs is described here : https://matplotlib.org/stable/users/explain/figure/api_interfaces.html

Example for distributions

For example, in the base class for distributions, the view method :

https://github.com/HarrisonKramer/optiland/blob/6530b818bdac359858bcd0368fa950e5c53b7476/optiland/distribution.py#L58

uses plt for accessing matplotlib. This code could be replace with, for example :

Before

    def view(self):
        """Visualize the distribution.

        This method plots the distribution points and a unit circle for
            reference.
        """
        plt.plot(self.x, self.y, "k*")
        t = np.linspace(0, 2 * np.pi, 256)
        x, y = np.cos(t), np.sin(t)
        plt.plot(x, y, "r")
        plt.xlabel("Normalized Pupil Coordinate X")
        plt.ylabel("Normalized Pupil Coordinate Y")
        plt.axis("equal")
        plt.show()

After

    def view(self, ax=None):
        """Visualize the distribution.
    
        This method plots the distribution points and a unit circle for
            reference.
        """
        if ax is None:
            fig, ax = plt.subplots()
        
        ax.plot(self.x, self.y, "k*")
        t = np.linspace(0, 2 * np.pi, 256)
        x, y = np.cos(t), np.sin(t)
        ax.plot(x, y, "r")
        ax.set_xlabel("Normalized Pupil Coordinate X")
        ax.set_ylabel("Normalized Pupil Coordinate Y")
        ax.set_aspect("equal")
        return ax

This way :

• the user can either provide the Axes or let the method just create a new one with signature (self, ax=None):
• and return the Axes for further customization with return ax
• and delay the actual displaying by removing plt.show()

Generalization

This approach could (and should?) be applied throughout all plottings with matplotlib in the package:

• https://github.com/HarrisonKramer/optiland/blob/4000c03774e889cba85599c71669b2bba9d037d1/optiland/visualization/visualization.py#L48
• https://github.com/HarrisonKramer/optiland/blob/6530b818bdac359858bcd0368fa950e5c53b7476/optiland/psf.py#L107
• https://github.com/HarrisonKramer/optiland/blob/6530b818bdac359858bcd0368fa950e5c53b7476/optiland/wavefront.py#L421

Note on circles

I believe using

circle = plt.Circle((xc, yc), R, fill=False)
ax.add_patch(circle)

is considered a cleaner approach to plot a circle on an ax (removes the need for sampling `t = np.linspace(0, 2 * np.pi, 256)' )

[HarrisonKramer]

Hi @mocquin,

Thanks for opening this! Please keep the recommendations coming.

I completely agree with your logic and think it makes sense to update this across the package. I have run into a few issues during the development when using the implicit pyplot API. I will plan to update this. I don't expect the refactor will cause any huge issues, so should be a straightforward update.

Regards, Kramer

[lordpositron]

I'd be happy to help with this issue. It looks like a great opportunity to start contributing to the project. From what I can see in the code, most of the work is already done; it just seems that some return fig, ax statements are missing. Is anyone already working on it? Regards, Corentin

[HarrisonKramer]

Hi @lordpositron,

Thanks for offering to work on this! That would be great. No one is working on this yet, so you can proceed with it. There are still many functions using matplotlib with a call to show() in the package. I suppose the easiest way to find them will be to search for "plt.show()". Ultimately, all of these will likely need to be changed, though there could be exceptions. If you prefer not to update everything at once, you can focus on one or more modules at a time.

Best, Kramer

[lordpositron]

Hi @HarrisonKramer, Thanks, for the quick answer ! I started with two files : spot_diagram.py and ray_fan.py, by removing the plt.plot() and returning the fig, ax objects. In interactive python/Jupyter, the figure behavior remains unchanged. However, in scripts, nothing will be displayed unless the returned figure is explicitly shown or saved.Let me know if there's anything I should consider before proceeding with the rest. https://github.com/lordpositron/optiland/commit/78c05f130f5884934e1d09dc747a1c96c5dc9914

Corentin

[HarrisonKramer]

Hi @lordpositron,

Thanks for the quick update. I glanced at your update and it looks good. The only thing that may be missing is adding a new argument ax=None to the view functions, but this currently conflicts with some of the GUI logic, so I propose we don't change that yet. I spoke with @manuelFragata, who will investigate that further. For now, returning the the fig and ax should be sufficient.

Thanks, Kramer

[lordpositron]

Hi @HarrisonKramer, I think I've done most of the groundwork by adjusting the logic in the view() methods. I also tried to integrate the GUI logic based on the existing implementation. I can open a pull request if it looks okay at first glance. Thanks, Corentin

[HarrisonKramer]

Hi @lordpositron, that's great. Thanks!

I took a quick glance at the branch and I think all looks good. You can open the PR and we can check that all tests pass before merging.

Thanks again, Kramer

[lordpositron]

Hi @manuelFragata, To address this issue, I slightly modified the display behavior of IncoherentIrradiance.view(). Instead of generating a separate figure for each wavelength and field, I now create a grid of subplots to produce a single combined figure. Let me know if this change interferes with the GUI behavior in any way.

Best regards, Corentin

[manuelFragata]

Hi @manuelFragata, To address this issue, I slightly modified the display behavior of IncoherentIrradiance.view(). Instead of generating a separate figure for each wavelength and field, I now create a grid of subplots to produce a single combined figure. Let me know if this change interferes with the GUI behavior in any way.

Best regards, Corentin

Hi @lordpositron ,

All good, no worries. for now, the irradiance analysis is not accessible from the GUI - I disabled this because I am still working on improvements to that analysis, and another upcoming one (intensity). I'll check that later, but so far, no problems. If there are any problems, I can fix them in the future, no worries :)

Thanks a lot for this contribution!

Best, ~Manuel

[HarrisonKramer]

This issue was resolved in #242. Closing now.