You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
uses plt for accessing matplotlib. This code could be replace with, for example :
Before
defview(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
defview(self, ax=None):
"""Visualize the distribution. This method plots the distribution points and a unit circle for reference. """ifaxisNone:
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")
returnax
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:
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 implicitpyplot API. I will plan to update this. I don't expect the refactor will cause any huge issues, so should be a straightforward update.
Most use of
matplotlib
througout the package rely on the implicitpyplot
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 :optiland/optiland/distribution.py
Line 58 in 6530b81
uses
plt
for accessingmatplotlib
. This code could be replace with, for example :Before
After
This way :
Axes
or let the method just create a new one with signature(self, ax=None):
Axes
for further customization withreturn ax
plt.show()
Generalization
This approach could (and should?) be applied throughout all plottings with
matplotlib
in the package:optiland/optiland/visualization/visualization.py
Line 48 in 4000c03
optiland/optiland/psf.py
Line 107 in 6530b81
optiland/optiland/wavefront.py
Line 421 in 6530b81
Note on circles
I believe using
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)' )
The text was updated successfully, but these errors were encountered: