Use magick command instead of the deprecated convert if available

[antonio-rojas]

The convert command is deprecated and throws a warning at runtime.

[github-actions]

Documentation preview for this PR (built with commit 64eaf5a; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

[dimpase]

why not rename convert to magick ?

[dimpase]

more precisely, ImageMagick version 7 provides magick, earlier versions only provide convert

[antonio-rojas]

If we want to drop support for IM6 then we shold replace all uses of convert throughout sage source, which would be a much larger change. I'm fine with that, but in the meantime this will fix the test failure. Changing to magick only for this particular instance doesn't seem right to me.

[jhpalmieri]

From a quick glance at imagemagick.org, it seems like we could replace convert ... with magick convert ... — the latter has been supported since version 6 of ImageMagick. Unfortunately I can't tell when version 6 was released and whether it is reasonable to expect version 6 or later to be supported on our standard platforms. Judging from https://legacy.imagemagick.org/discourse-server/viewtopic.php?t=32622, it looks like version 6 is pretty old and should be well-established.

I guess my question is, should we switch away from the deprecated convert instead of just hiding the deprecation warning with ...?

[antonio-rojas]

From a quick glance at imagemagick.org, it seems like we could replace convert ... with magick convert ... — the latter has been supported since version 6 of ImageMagick.

ImageMagick 6 doesn't install any magick executable. And in any case, magick convert throws the same deprecation warning as convert in 7.1.1.33, so it wouldn't solve anything.

[dimpase]

we can test for magick in spkg-configure. And instead of convert use something we'd set up there, to be either convert or magick.

And fix src/sage/features/imagemagick.py accordingly, too.

[dimpase]

sage.features for ImageMagick, and spkg-configure, should be updated
(I can help with this, if needed)

[dimpase]

it seems like we could replace convert ... with magick convert ...

Isn't the correct and stable new syntax merely magick ..., not magick convert ... ?
I tried magick foo.jpg foo.pdf, it works fine with magick version 7.1.1-33.
(and if I use convert instead, I get the warning)

[jhpalmieri]

it seems like we could replace convert ... with magick convert ...

Isn't the correct and stable new syntax merely magick ..., not magick convert ... ? I tried magick foo.jpg foo.pdf, it works fine with magick version 7.1.1-33. (and if I use convert instead, I get the warning)

I'm sure you're right. As I said, I just took a quick glance at the website, and I guess I misunderstood what they meant by "supporting subcommands for compatibility with ... version 6" (https://imagemagick.org/script/command-line-tools.php).

[antonio-rojas]

I haven't (and don't plan to) touched anything outside src, in particular spkg-configure.m4. This will keep working fine until the convert command is removed, so it can be fixed in a later ticket.

[dimpase]

it seems that your patch assumes presence of magick - but it's not there if an old version of ImageMagick is installed. There you only have convert.

[antonio-rojas]

it seems that your patch assumes presence of magick - but it's not there if an old version of ImageMagick is installed. There you only have convert.

No, it falls back to convert if magick is not available. This has been tested with both IM 6 and 7.

[dimpase]

OK, sorry, I misread the diff.

[dimpase]

please rebase over the latest develop branch (it goes cleanly)

[GMS103]

FWIW, positive review for SageMath 10.4.beta9 on an Apple Silicon Mac M1 Pro with Homebrew up to date, on macOS 12.7.5 (Xcode 14.2).

[mkoeppe]

CI failures are unrelated. Let's merge it, although a cleaner implementation as discussed in #38173 would be preferable.

[GMS103]

Thanks.