Using PNGFiles.jl via ImageIO.jl (lazy loader) for .png image files

[IanButterworth]

https://github.com/JuliaIO/PNG.jl isn't yet registered, but is functional with this branch.

[codecov]

Codecov Report

Merging #257 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #257   +/-   ##
=======================================
  Coverage   76.08%   76.08%           
=======================================
  Files           8        8           
  Lines         414      414           
=======================================
  Hits          315      315           
  Misses         99       99

Impacted Files	Coverage Δ
src/registry_setup.jl	92% <ø> (ø)	⬆️
src/registry.jl	92.64% <ø> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 67d9c22...c408858. Read the comment docs.

[IanButterworth]

As @davidanthoff pointed out on slack, we need to make sure the MIME for png isn't messed up by this. Just making a note as a reminder

[timholy]

Need to wait for JuliaRegistries/General#9508 (3 days).

[johnnychen94]

Does this mean that to load a png file we'll need to install PNG instead of ImageMagick? If so then a lot of packages' test codes will be broken without adding PNG to the Project.toml's extra section.

[IanButterworth]

Need to wait for JuliaRegistries/General#9508 (3 days).

Indeed, and as @johnnychen94's point illustrates, even once that's registered this needs careful thinking before merging

[IanButterworth]

It makes me think that if we were to move to this approach, it might be worth waiting until the big three, PNG.jl, JPEG.jl and TIFF.jl, were ready to make the switchover easier?

[timholy]

IIRC packages get precedence by argument order. So we could leave QuartzImageIO and ImageMagick as valid loaders but put PNG as the first choice.

[johnnychen94]

If the final goal is to deprecate ImageMagick(is it?), then I think the following procedure makes sense:

1. add a deprecation warning in ImageMagick and QuartzImageIO for specific image format (e.g., .png to encourage users to try out PNG.jl
2. leave ImageMagick and QuartzImageIO as fallbacks in FileIO for a period
3. unregister ImageMagick and QuartzImageIO from FileIO

[timholy]

I think we're likely to have ImageMagick around for some time for less commonly-used formats. Just restoring those lines to the add_format seems like it should be enough even without changing any of the other packages.

[IanButterworth]

Just wanted to highlight that I thought this was a typo, so corrected it

[davidanthoff]

Could you try whether this would break the existing MIME based PNG story? Here is a simple test:

using VegaLite, VegaDatasets

dataset("cars") |> @vlplot(:point, x=:Acceleration) |> save("foo.png")

[IanButterworth]

@davidanthoff does this look right?

[davidanthoff]

Yes, if it produces a proper graph, all is good :)

[timholy]

Just strip the WIP out of the title if you're ready for this to be merged.

EDIT: that might not send a notification, so p(i)ng me too. (bad pun intended)

[IanButterworth]

Will do. Need to wait for ImageIO to be released and a plan to be settled on for the test failures in JuliaIO/PNGFiles.jl#5

[IanButterworth]

@timholy Ready to merge, given JuliaIO/PNGFiles.jl#5 has been merged and released (edit: had the wrong PR linked)

[timholy]

Oh boy oh boy oh boy!