image_data Serialization

[jupyter-widgets-contrib]

image_data Serialization

[martinRenou]

Thanks for opening an issue!

I am surprised I did not use this.ctx.getImageData in the first place. I hope I did not have a good reason to not do so. I totally agree with your implementation, if you can come up with a PR I would be happy to merge it! Also removing the Pillow dependency is good.

Though I think an even more general solution is to use the serializers and traittypes from https://github.com/vidartf/ipydatawidgets

I disagree with this because I don't want ipycanvas to depend on another widgets library. Having users to install one labextension/nbextension is already quite difficult, so asking them to install two of them in order for ipycanvas to work is too much IMO.

[martinRenou]

It would be nice to compare performances between your solution and the previous one though. I might have used the toBlob for performances purpose, but I am not sure.

[ianhi]

I disagree with this. I don't want to depend on another widgets library. Having users to install one labextension/nbextension is already quite difficult, so asking them to install two of them in order for ipycanvas to work is too much IMO.

100% that installing more than one labextension is no good. Though I don't think that would be necessary. It's not clear from the name but that is broken into separate packages so you can have the traittype and serializers without requiring any jupyter labextension install steps. There is 1 python package and two different js packages. 1 of them is the extension and widgets, and the other is just the serializers. (related is this issue jupyter-widgets/traittypes#21)

It would be nice to compare performances between your solution and the previous one though. I might have used the toBlob for performances purpose, but I am not sure.

Maybe size was your reason? see: https://stackoverflow.com/a/34924715/835607. given the differences in size maybe it's best to leave as is?

I think that the ndarray traittype gets around by offering a compression option https://github.com/vidartf/ipydatawidgets/blob/69eebeb5647d58dec6bf459d424727cb23cce8ab/packages/serializers/src/ndarray.ts#L167

[ianhi]

Is performance also the reason you have sync_image_data default to False?

[martinRenou]

Though I don't think that would be necessary. It's not clear from the name but that is broken into separate packages so you can have the traittype and serializers without requiring any jupyter labextension install steps. There is 1 python package and two different js packages. 1 of them is the extension and widgets, and the other is just the serializers. (related is this issue jupyter-widgets/traittypes#21)

Good catch! Then I agree we could depend on the Python package to make things modular :)

Maybe size was your reason? see: https://stackoverflow.com/a/34924715/835607. given the differences in size maybe it's best to leave as is?

Yes that might have been the reason for not using getImageData directly. Although if you think the compression that happens during the blob transformation is a problem (loss of data?) we should maybe consider using getImageData instead.

Is performance also the reason you have sync_image_data default to False?

Indeed.

[martinRenou]

Is performance also the reason you have sync_image_data default to False?

I might be wrong, but I thought that in most of the cases, people would not need to retrieve the image as a NumPy array.

[ianhi]

Although if you think the compression that happens during the blob transformation is a problem (loss of data?) we should maybe consider using getImageData instead

I think PNG compression is lossless and the conversions that you're using result in PNG compression so that shouldn't be an issue. If you wanted an intermediate to using ipydatawidgets then you can trade the Pillow dep for a typescript one (pako) (like this ianhi/ipysegment@0127f5b) and use the python builtin zlib. So I guess in the end it's kind of a wash.

I might be wrong, but I thought that in most of the cases, people would not need to retrieve the image as a NumPy array.

That also rings true

[martinRenou]

Should we close this?

[ianhi]

Probably. Feel free to re-open if you ever want to change your image data serialization strategy, but what you have now works, so there's no real need to change it.