search

MBravoS / splotch

Simple PLOTs, Contours and Histograms is a small package with wrapper functions designed to simplify plotting calls from matplotlib.
BSD 3-Clause "New" or "Revised" License
5 stars 0 forks source link

Changing xlim/ylim=None behaviour #72

Closed AstroRobin closed 1 year ago

AstroRobin commented 1 year ago

On a related note to #66 where grid was being overwritten by subsequent splotch calls, a similar thing occurs with xlim/ylim. Because setting xlim = None triggers the limits to be set by autoscale, this forces the user to always specify their limits in their final splotch call. I think this is a bad design because whenever a new splotch call is made, this requires either rearranging function order, using the zorder parameter or rewriting code.

Instead, I think we take a similar approach to the solution for grid parameter, whereby we first check if this is the first plot call and if so then setting xlim = None does indeed trigger autoscale and if not, then setting xlim = None does not change limits, because those limits have probably already been set.

This might create an edge case where the user initiates their first plot with xlim=None (i.e. autoscale), meaning that the auto limits are retained for all subsequent plot calls if the user continues with xlim=None and perhaps this first plot call only covers a narrow range? I still think this is better than overwriting previously user-specified limits.

AstroRobin commented 1 year ago

I've implemented a version of this behaviour in https://github.com/MBravoS/splotch/commit/3c63750775cc54a7f8105b8695333c5705ab9249 for functions in plots_1d.py as these are the simplest.

This feature required the addition of the lims_handler() function in base_func.py, which standardises the user's xlim/ylim values across all functions. It also required a slight adjustment to _plot_finalizer() in which xlim/ylim can also have the state 'auto' — technically the user can also specify xlim='auto' and I think in future this could be added to default Params fairly easily.

Side note, if one passes None into matplotlib's plt.set_xlim(None) function, then nothing happens as one might expect. The new behaviour of xlim/ylim mimics this functionality, keeping it in line with user expectations.

MBravoS commented 1 year ago

Nice! It seems to be working nicely, so I think that once we converge on #66 I'll combine the changes in that issue, this, and those for #73 into one release.

AstroRobin commented 1 year ago

Nice! It seems to be working nicely, so I think that once we converge on #66 I'll combine the changes in that issue, this, and those for #73 into one release.

Sounds like a good plan!