Pillow built without jpeg support in Sage 10.2.rc3

[Eric Gourgoulhon]

Hi All,

Pillow provides the Python Imaging Library (PIL)

https://pillow.readthedocs.io/en/stable/

and is a standard package in Sage. 

Now, in Sage 10.2.rc3, the following code

g = plot(sin(x))     # to generate a png figure
g.save("fig.png")  #

from PIL import Image
img = Image.open("fig.png")

img_rgb = img.convert("RGB")

img_rgb.save("fig.jpg")

returns

OSError: encoder jpeg not available

This is actually not surprising since the file

SAGE_ROOT/build/pkgs/pillow/spkg-install.in

contains the line:

PILLOW_CONFIG_SETTINGS="-C debug=true -C jpeg=disable $PILLOW_CONFIG_SETTINGS"

Is there any reason for "jpeg=disable" ? 

Providing PIL without jpeg support seems quite weird...

This issue is not new and was already noticed already 5 years ago:

https://ask.sagemath.org/question/43298

A side effect of this is that the IPyhon  rendering of PIL Images via the rich output mechanism is broken in Sage 10.2.rc3 (but, for some reasons, not in Sage 10.1): 

in a Jupyter notebook, typing

img = Image.open("fig.png")

img

does display the image but after an error message terminating by 

ValueError: Could not save to JPEG for display

There is no such error message in Sage 10.1

If we agree to restore jpeg support in Pillow, I have prepared a branch that does this for Sage 10.2 (simply suppressing the option "jpeg=disable"):

https://github.com/egourgoulhon/sage/tree/pillow_jpeg

and I am happy to submit a PR for this.

Eric.

[David Roe]

I'm not aware of why we were suppressing jpeg support before, but this sounds like a good idea to me.

David

--
You received this message because you are subscribed to the Google Groups "sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sage-devel/e1a2bf34-d3e5-492b-942c-33b1fc8c3254n%40googlegroups.com.

[Matthias Koeppe]

On Thursday, November 16, 2023 at 6:48:47 AM UTC-8 Eric Gourgoulhon wrote:

[...] the file

SAGE_ROOT/build/pkgs/pillow/spkg-install.in

contains the line:

PILLOW_CONFIG_SETTINGS="-C debug=true -C jpeg=disable $PILLOW_CONFIG_SETTINGS"

Is there any reason for "jpeg=disable" ? 

This was done in 2016, see https://github.com/sagemath/sage/issues/20021#issuecomment-1417764945, apparently for platform support reasons

Providing PIL without jpeg support seems quite weird...

I agree

 

This issue is not new and was already noticed already 5 years ago:

https://ask.sagemath.org/question/43298

A side effect of this is that the IPyhon  rendering of PIL Images via the rich output mechanism is broken in Sage 10.2.rc3 (but, for some reasons, not in Sage 10.1)

This temporarily worked accidentally because Pillow had made an incompatible change to the format of its configuration settings. I repaired that as part of the Pillow 10 upgrade in https://github.com/sagemath/sage/pull/36320

 

If we agree to restore jpeg support in Pillow, I have prepared a branch that does this for Sage 10.2 (simply suppressing the option "jpeg=disable"):

https://github.com/egourgoulhon/sage/tree/pillow_jpeg

and I am happy to submit a PR for this.

Yes, please. 

 

[Michael Orlitzky]

On Thu, 2023-11-16 at 06:48 -0800, Eric Gourgoulhon wrote:
>
> If we agree to restore jpeg support in Pillow, I have prepared a branch
> that does this for Sage 10.2 (simply suppressing the option "jpeg=disable"):
> https://github.com/egourgoulhon/sage/tree/pillow_jpeg
> and I am happy to submit a PR for this.
>

That makes pillow link to libjpeg, so we'd also need to package some
incarnation of libjpeg, make it a standard package, write the spkg-
configure script, etc.

AFAIK the "best" libjpeg is https://libjpeg-turbo.org/, but then we
would also need to package an assembler (nasm or yasm) to build it...

[Dima Pasechnik]

let's not make libjpeg a real package.
Make it a dummy package, so that libjpeg may be advertised as something to be installed.

IIRC the difficulty in packaging jpeg was one of yhe reasons it never made it to a package.

>

[Matthias Koeppe]

Or we just let pillow use a system libjpeg if it finds one.

[Dima Pasechnik]

On 17 November 2023 04:35:53 GMT, Matthias Koeppe <matthia...@gmail.com> wrote:
>Or we just let pillow use a system libjpeg if it finds one.

With a dummy package we can have doctests,
with #optional tag, checking that this all works.

[Eric Gourgoulhon]

Le vendredi 17 novembre 2023 à 10:14:54 UTC+1, Dima Pasechnik a écrit :

On 17 November 2023 04:35:53 GMT, Matthias Koeppe <matthia...@gmail.com> wrote:
>Or we just let pillow use a system libjpeg if it finds one.

This would probably be the easiest solution: I cannot imagine a system without libjpeg...

 

With a dummy package we can have doctests,
with #optional tag, checking that this all works.

What is exactly a "dummy package"? Is it something that requires a system package ?  Is it complicated to set up and maintain? 

Otherwise, we could just require libjpeg as a prerequisite to build Sage from source and add it to the list

https://doc.sagemath.org/html/en/installation/source.html#prerequisites

In any case, at the moment, PIL is barely usable in the Sage 10.2.rc3  notebook, even if no jpeg image is manipulated. For instance,  the following code considers only a png image:

from PIL import Image
plot(sin(x)).save("fig.png")

img = Image.open("fig.png")
img

  

and yields

ValueError: Could not save to JPEG for display

There was no such issue in previous versions of Sage (e.g. 10.1). 

As a consequence, the following public notebook 

https://nbviewer.org/github/sagemanifolds/SageManifolds/blob/master/Notebooks/SM_black_hole_rendering.ipynb

is broken in Sage 10.2.rc3 and there are probably many of those. 

Eric.

[Michael Orlitzky]

On Fri, 2023-11-17 at 02:33 -0800, Eric Gourgoulhon wrote:
> On 17 November 2023 04:35:53 GMT, Matthias Koeppe <matthia...@gmail.com>
> wrote:
> > Or we just let pillow use a system libjpeg if it finds one.
>
> This would probably be the easiest solution: I cannot imagine a system
> without libjpeg...

I don't know how common it is, but it's pretty easy to build a headless
server without libjpeg.

[Matthias Koeppe]

On Friday, November 17, 2023 at 1:14:54 AM UTC-8 Dima Pasechnik wrote:

On 17 November 2023 04:35:53 GMT, Matthias Koeppe <matthia...@gmail.com> wrote:
>Or we just let pillow use a system libjpeg if it finds one.

With a dummy package we can have doctests,
with #optional tag, checking that this all works.

No, that's not true. 

1) For dummy packages, there is no stamp file in local/var/lib/sage/installed/, which is where our "# optional" tags have traditionally come from. For example, pandoc is a dummy package, and you won't see it there whether it's installed in the system or not.

2) "# optional" tags can be established by runtime Feature tests. Again, with the example of pandoc, see https://doc.sagemath.org/html/en/reference/spkg/sage/features/pandoc.html

 

[Matthias Koeppe]

On Friday, November 17, 2023 at 2:33:06 AM UTC-8 Eric Gourgoulhon wrote:

What is exactly a "dummy package"? Is it something that requires a system package ?  Is it complicated to set up and maintain? 

It's a package for which the Sage distribution does not provide a method of installing it, but may provide information on system packages (example: https://doc.sagemath.org/html/en/reference/spkg/pandoc.html#spkg-pandoc) and a script for testing whether a suitable system package is present (spkg-configure). 

See https://doc.sagemath.org/html/en/developer/packaging.html#package-source-types (item 5)

 

[Dima Pasechnik]

why do we even need runtime Feature tests? This is a leftover from the
times one could install Sage packages in a Sage session, something
that is long gone,
as far as I know.

Why can't Features be set up by ./configure ? You'll tell me that
(some) distributions don't run ./configure, but this is beside the
point.
They have ways to know what's installed and what's not installed.

>
>
> --
> You received this message because you are subscribed to the Google Groups "sage-devel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/sage-devel/3dea5580-39b4-4033-95a3-0c826cc2860fn%40googlegroups.com.

[Matthias Koeppe]

On Friday, November 17, 2023 at 9:27:03 AM UTC-8 Dima Pasechnik wrote:

why do we even need runtime Feature tests? This is a leftover from the
times one could install Sage packages in a Sage session

No, it's not. It's a much newer facility.

[Dima Pasechnik]

Anyhow, I don't see its purpose. We have at least one open issue/PR,
adding spkg-configure.m4 for FriCAS,
stuck due to the fact it's an insanely complicated job to provide
Feature for it, and totally meaningless, if you ask me.

>
> --
> You received this message because you are subscribed to the Google Groups "sage-devel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/sage-devel/b7b7c690-d039-4fd0-ac61-b04049a38030n%40googlegroups.com.

[Matthias Koeppe]

On Friday, November 17, 2023 at 10:11:43 AM UTC-8 Dima Pasechnik wrote:

We have at least one open issue/PR,
adding spkg-configure.m4 for FriCAS,
stuck due to the fact it's an insanely complicated job to provide
Feature for it, and totally meaningless, if you ask me.

I'll interpret this declaration as you asking for help with this PR, https://github.com/sagemath/sage/pull/35838, so I have put it on my list of things to do.

[Marc Culler]

I expect to receive lots of flak for saying this, but I support making libjpeg be a standard spkg using the source code from https://libjpeg.sourceforge.net.  I just built version jpeg-9e on Ubuntu 18.04 and macOS 10.13.  The standard ./configure ; make install method works flawlessly - not even any warnings.  The build is fast and the package is small.  Installing it in sage/local/lib will be simpler, faster and more reliable than guessing where it might be found on any of the zillion systems that Sage runs on.  Also, unlike  the turbo alternatives, it does not depend on fancy features of Intel hardware which will not be available on old Intel CPUs or recent Arm CPUs.

- Marc

[Marc Culler]

I meant "normal" when I said "standard".

[Marc Culler]

No, I meant both "normal" and "standard".  I am sure I will master this terminology sooner or later.

[Marc Culler]

I should also have mentioned that macOS is an example of a system which does not come with libjpeg.

--
You received this message because you are subscribed to a topic in the Google Groups "sage-devel" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/sage-devel/A4nY3bh1oPI/unsubscribe.
To unsubscribe from this group and all its topics, send an email to sage-devel+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sage-devel/ca9fc171-9f04-4437-bc4a-0471e95dfef4n%40googlegroups.com.

[Michael Orlitzky]

On Fri, 2023-11-17 at 17:26 +0000, Dima Pasechnik wrote:
>
> Why can't Features be set up by ./configure ? You'll tell me that
> (some) distributions don't run ./configure, but this is beside the
> point.
> They have ways to know what's installed and what's not installed.
>

I think the problem is that ./configure is in the wrong place. Sagelib
needs its own build system that can detect libraries, run tests, and
determine paths, so that things still work when we build only sagelib.

That's one of the reason's I'm so happy that Tobias is working on the
meson build. In theory it would let us eliminate most of the top-level
./configure, sage-conf, and all of the runtime feature testing -- to be
replaced with something simple, predictable, and standard.

[Michael Orlitzky]

On Fri, 2023-11-17 at 10:49 -0800, Marc Culler wrote:
> I expect to receive lots of flak for saying this, but I support making
> libjpeg be a standard spkg using the source code from
> https://libjpeg.sourceforge.net. I just built version jpeg-9e on Ubuntu
> 18.04 and macOS 10.13. The standard ./configure ; make install method
> works flawlessly - not even any warnings. The build is fast and the
> package is small. Installing it in sage/local/lib will be simpler, faster
> and more reliable than guessing where it might be found on any of the
> zillion systems that Sage runs on. Also, unlike the turbo alternatives,
> it does not depend on fancy features of Intel hardware which will not be
> available on old Intel CPUs or recent Arm CPUs.

I was vague about this because I didn't want to go digging...

We had to drop this from Gentoo a few years ago for several reasons.
The first is simply that it's not actively maintained. There are
roughly two years between versions, which isn't good enough when a new
clang is released every few months. Where's the bug tracker? Mailing
list? People can't quietly accept security issues or build failures for
that long. We could work around it by backporting enough patches, but
do we really want to maintain a sage fork of libjpeg for two years at a
time? (Are we going to write the patches ourselves if the other distros
quit doing it? Is any distro still shipping the IJG libjpeg?)

The second is that it's no longer compatible with libjpeg-turbo, and
that's what most people target. Chromium and Qt, for example, only
build against libjpeg-turbo -- or at least did, back when we removed
libjpeg. Thus if you have to pick only one and ship it to your users,
the choice is obvious. You can't have both because they provide the
same API. This might not be a huge problem so long as pillow and every
other sage component support them both, but who knows what the future
holds.

The libjpeg-turbo features degrade gracefully by the way. I'm certain
to have the oldest intel computer here, and I've been using libjpeg-
turbo for 9+ years, as far back as my git history goes.

I'm not against adding jpeg support to sage per se, I just think it's
going to be a pain to add it to sage-the-distribution. I'm using my
system's copy of pillow that uses my system's copy of libjpeg-turbo,
nicely avoiding the entire problem, but only as a user.

[Matthias Koeppe]

If we decide to make libjpeg a normal standard package, then I'd be +1 on using libjpeg-turbo. Debian also switched to it many years ago, and it only has to apply a small amount of patches (https://sources.debian.org/src/libjpeg-turbo/1%253A2.1.5-2/), which we could easily adopt.

[Matthias Koeppe]

PR using this approach at https://github.com/sagemath/sage/pull/36731