OpenDrift / trajan

Trajectory analysis package for simulated and observed trajectories
https://opendrift.github.io/trajan/
GNU General Public License v2.0
11 stars 5 forks source link

plot trajan spectra with optional ax argument #134

Open hevgyrt opened 1 week ago

hevgyrt commented 1 week ago

Is it possible to add an optional ax argument to the plot_trajan_spectrahttps://github.com/OpenDrift/trajan/blob/main/trajan/plot/spectra.py method?

I think this would be convenient for simple comparison against model results, like

fig,ax = plt.subplots(nrows=2,sharex=True,sharey=True)

model_1d_spectra.plot(ax=ax[0],x='time')
plot_trajan_spectra(ds_obs.isel(trajectory=range(9,10)),ax=ax[1])
jerabaul29 commented 1 week ago

I can do a PR for this today :) . What do you think would be best @gauteh : forwaring all kwargs to the plot call? having an extra argument tuple (fix, ax)=('None','None') that can be ovewritten by the user to put their own?

jerabaul29 commented 1 week ago

(looks from other trajan function like forwarding *args* , **kwargs is the way to go? I can copy the API / logics / way of thinking of the trajan.plot.lines / .scatter :) ).

gauteh commented 1 week ago

Hi,

this should be done like in: https://github.com/OpenDrift/trajan/blob/main/trajan/plot/__init__.py#L138, then we are aligned with xarray. They also support giving an ax argument, if it is not present a new axis is created. If one exists, but is not specified, it is used. If you need a new one you create one manually (just like regular matplotlib).

Figure saving should not be part of the function, but should be outside it.

I think this should go in a variable accessor on the spectra variable. E.g.: ds.E.waves.plot()

-- gaute

gauteh commented 1 week ago

@jerabaul29 should I set up the accessor? then I can leave the rest of the tweaks to you?

gauteh commented 1 week ago

@jerabaul29 See #137.