Suggestion for xarray integration

[TomNicholas]

A simple example showing some of the advantages of providing convenience functions for plotting xarray DataArrays. xarray integration means that you could streamline the visualisation of time-evolving data down to just a few lines, e.g.:

data = xr.load_dataset('3d_data.nc')
anim, block, timeline = amp.animated_plot(da)
anim.save_gif('./xarray_imshow')
plt.show()

Although I think full seamless integration into xarray's system would be quite involved (and not something I'm qualified to do), then providing a set of convenience functions something like this shouldn't be very difficult.

Notice there are several # TODOs in the code showing how this could be extended.

Let me know what you think!

[t-makaro]

Thank you for the contribution. I'll take a closer look tonight, but I'd like some changes.

xanim.py should be within the animations folder. I don't think it should have a top level import. Any convenice functions like these should be available in animatplot.animations.*, making this
from animatplot.animations.xanim import .... Which shouldn't require any change to any __init__.py.

Currently, examples are in the docs/source/gallery/ directory as Jupyter notebooks. I plan on moving the gallery to sphinx-gallery in the feature, so you could change it to a notebook (the notebooks have a particular format and raw cells), or move the examples folder up a directory to the root where docs, tests, and Animatplot live, which is where I'll put examples when I switch to sphinx-gallery.

I don't want a hard dependency on xarray. We'll need to figure out optional dependencies. I've never used them before. If xanim.py, isn't imported at import time (by default), then something like

try:
    import xarray ...
except ImportException as e:
   raise ...

should be sufficient.

[t-makaro]

The core of xarray is still a numpy array. I sort of think that anim_over should be t_axis. That said, the xarray docs keep using the term "coods", so maybe t_coord? I think I prefer t_axis for api consistency. OTOH, t_axis is usually an integer....

Either way, the default value should be None, and you can check

if anim_over is None:
...

Also, could you add the numpydoc style parameter to the docstring? Looks like:

Parameters
------------
parameter_name : type
    description
…
Returns
--------
type1
    description
type2
   description

You can read about numpydoc here

[TomNicholas]

I sort of think that anim_over should be t_axis.

That would defeat a lot of the purpose of using xarray. The point of xarray is that the dimensions of the data are labelled in a human-readable manner, and you can interact with the data without specifying seemingly-arbitrary numbers (as in axis=0).

the xarray docs keep using the term "coords"

The xarray docs use the term coord because that is one of the fundamental objects in xarray, and it's not the same thing as a dimension (which is not the same thing as an axis!).

To be honest when I first started using xarray I was like "is all this extra jargon really necessary?", but having used it for a while it makes way more sense.

the default value should be None

Yep you're right.

I think I prefer t_axis for api consistency

I didn't use the word "axis" because that implies an integer numpy axis, which is what xarray allows you move past. I didn't use the letter "t" because t_axis only represents time in the sense that a gif evolves over real time. In general then t_axis could refer to something which isn't time at all. For example, I could have a DataArray of temperature T(x,y,z,t) in the atmosphere for each day in a month. If I want to plot a gif of the variation of temperature with height ona particular day then it would be much clearer to be able to write animate(da, anim_over='z') or even animate(da, anim_over='height') then to write animate(da, t_axis='z') (or worse, animate(da, t_axis=3). I chose "anim_over" because it's literally the dimension of the data over which the gif is to be animated. What do you think?

numpydoc

Yep, I am more than happy to conform to animatplot's commenting and documenting style (and write some unit tests), I just wanted to get the general idea of the layout worked out first.

[t-makaro]

Ok, anim_over it is.

[t-makaro]

I really like the way you return things here. It bodes well for future functions like this. But, I sort of think it should be a list of blocks (because future convenience functions could return multiple blocks). OTOH, advanced tuple unpacking

anim, *blocks, timeline = ...

is a thing. OTOH,

anim, [block1, block2], timeline = ...

is also possible. That's a tough decision. Keep with this and I'll make a decision befor 0.3 release.

[TomNicholas]

I personally like the

anim, [block1, block2], timeline = ...

variant more, but this is something that would be easy to change anyway.

[t-makaro]

Easy to change, but annoying to change after release. Ya, make it return a list of blocks. It will be easier to explain in the docs too.

[t-makaro]

remove the print statements please

[t-makaro]

Should this say "only plots 3D dataarrays"?

[TomNicholas]

Well it currently only plots 2D DataArrays which evolve over a 3rd dimension... Not sure what the clearest way to write that is.

[t-makaro]

"Only animates 3D arrays"?

[t-makaro]

So, I added a vector_plot convenience function in #3. I'm questioning now whether having these convenience functions make the call to animatplot.Animation and return the object is a good idea or not. It stops them from being composed with other blocks.

If they just return a list of blocks (and sometimes a timeline as in your case), then potentially other blocks could be used with them to make the animation. OTOH, if they don't make a complete animation then they need to have an axis parameter to tell them what matplotlib axis to attach to (with a default to None).

Sometimes, I feel like I'm developing faster than I can think. What do you think?

[TomNicholas]

I added a vector_plot convenience function in #3

That was quick!!

they need to have an axis parameter

Creating a complex plot by creating each block one by one is a good system in my opinion. Can the information about the axis they need to go on not be encoded in the block itself?

I don't want a hard dependency on xarray.

I've been thinking, and I'm now wondering whether it would make more sense to have xarray wrap animatplot than have animatplot wrap xarray... I've submitted an issue to xarray here which explains my thought process...

[t-makaro]

Ok, so I just did a new release. I added an animations subpackage, and a new concept of composition blocks. I think that we should make a composition block for this that returns a list of blocks and a timeline, then we can make a wrapper around that within the animations subpackage that produces the animation.

That only thing that we need to figure out is how to make xarray an optional dependancy. I'm happy to have this exist within animatplot.

[TomNicholas]

So I now think that it makes more sense to wrap animatplot with xarray then to wrap xarray with animatplot. I'm closing this because any future coupling should be an extension of what I've started in this PR to xarray.