iris icon indicating copy to clipboard operation
iris copied to clipboard

Add dask array html repr to cube html repr

Open DPeterK opened this issue 6 years ago • 11 comments

Extend the cube html repr to include the html repr of the cube's lazy dask array, if present. If the cube is not lazy or the dask version does not support html repr of arrays then the dask array repr is skipped without erroring.

Here's an example: Screenshot 2019-08-28 at 16 29 51

DPeterK avatar Aug 28 '19 15:08 DPeterK

It is not functioning for me, when I test with :

  • firefox
  • IPython 4.2.0
  • jupyter core 4.4 + client 5.3
  • dask 1.2.2

It seems that the dask arrays do not provide _repr_html_. What do we need to make this work ?

pp-mo avatar Aug 28 '19 16:08 pp-mo

What do we need to make this work ?

Dask 2.0 😉 (see the dask whatsnew)

@pp-mo when you say not working, do you mean that the entire cube repr fails (it shouldn't do that) or just that the dask array repr doesn't appear? The second is intentional, as per the description of this PR:

If ... the dask version does not support html repr of arrays then the dask array repr is skipped

Does that help?

DPeterK avatar Aug 29 '19 09:08 DPeterK

BTW ping @mrocklin - I've been looking forward to combining the Iris cube html repr with the dask array html repr since the dask repr was introduced!

DPeterK avatar Aug 29 '19 09:08 DPeterK

when you say not working, do you mean that the entire cube repr fails (it shouldn't do that) or just that the dask array repr doesn't appear?

No, it's just missing the dask repr part. I was using Python 2.7 <<slaps own wrist>> for certain convenience reasons. No Dask 2 in python 2 !

pp-mo avatar Aug 29 '19 10:08 pp-mo

the entire cube repr fails (it shouldn't do that)

Even so, my simple example frankly looks a bit horrible, and I'm not even clear whether it is "correct".

iris.tests.stock.istk.global_pp() 

notebook_screenshot

Obviously that might belong on another ticket

pp-mo avatar Aug 29 '19 10:08 pp-mo

Ooh no that doesn't look right. In fact it looks like there's a problem with the cube's summary string that means it's not being parsed correctly. I don't think that's related at all to the changes here; you'll probably find the same behaviour on upstream Iris...

Edit: in fact, I can't reproduce the behaviour you're seeing: Screenshot 2019-08-29 at 11 40 22

DPeterK avatar Aug 29 '19 10:08 DPeterK

@lbdreyer why v3 and not v2.3? These are only small changes...

DPeterK avatar Aug 29 '19 10:08 DPeterK

@lbdreyer why v3 and not v2.3? These are only small changes...

Admittedly I did that before properly looking at this PR. My thinking was along the lines of 'if it's not going to work in Python 2 then it could instead go into the Iris 3 release when we drop Python 2 support'. If it doesn't break under Python 2 then it can certainly go in before Iris 3.

We are under a little pressure with outstanding functionality/fixes that need to go in for Iris 2.3, so I can't guarantee this change will also get in in time, but I will take off the Iris 3 milestone/project.

lbdreyer avatar Aug 29 '19 13:08 lbdreyer

Thanks @lbdreyer!

DPeterK avatar Aug 29 '19 14:08 DPeterK

Just to put down some thoughts on the bug @pp-mo is seeing here, I had a look at the code and i think this is what is happening:

I believe somehow the data associated with the dimension coordinate also contains the information for scalar coordinates. At this line we begin treating this information as if we were expecting only '+' and '-' separated by whitespace. The body now consists of a list of all the 'words' and the colspan is set to 0. When making a row, we end up here, and we append each item in the list of words in the body as its own column. I'm still not sure what is causing this behaviour to occur, but I don't believe this would be caused by the changes made here.

stephenworsley avatar Sep 26 '19 09:09 stephenworsley

Hi @DPeterK I'm just considering this alongside #3376 and #3380 for inclusion in Iris 3.1.0

But I'm a bit doubtful now as to whether you/we can find time to fix these up in the next ~2-3weeks ? So I have just for now moved all 3 back to the 'Backlog' column of the 3.1.0 project.

I'm considering these 3 together, as they all concern the Cube._repr_html(). It's possible that #4206 may break any/all of these (thought it seems valid for now?) But in general anyway, we would now like to re-write the support based on #3987, which should make for mush easier future maintenance.

So , could you maybe update how you feel about this ? If you're keen, I'm pretty sure that #3376 and #3380 could in fact still go in, with probably just a little work from us. However this PR, at least, does still seem to need some more work.

pp-mo avatar Jul 30 '21 09:07 pp-mo

This seems to have gone stale. We still have ambitions for better HTML, particularly given @pp-mo's work turning the Cube summary into actual objects, which would make for much better HTML creation. But that would need a different PR, so I'm going to close this one. @DPeterK feel free to re-open if you disagree.

Specifically on this one: we LOVE how it looks, so definitely something to revisit in the new Cube summary world.

trexfeathers avatar Sep 21 '22 09:09 trexfeathers