Add mime save for eps, pdf, png and svg

[davidanthoff]

This is a first stab at #122 for some file formats. @SimonDanisch does this look good in terms of general direction? I've tested this with the current version of VegaLite.jl, and it seems to all work.

If you are ok with the general design here, I'll add tests and then we could merge?

[SimonDanisch]

Looks good! Do we have tests for this behaviour? I'd love to have some before merging. Makes reviewing easier ;) Btw, thanks for the effort!

[davidanthoff]

No tests yet, but I'll add them before we merge this.

[davidanthoff]

Alright, I think this is done from my point of view.

But this change will affect a lot of things, i.e. for any object that supports show for png, eps and pdf files this will change how they will be handled in the save code path. ImageMagick will no longer kick in, instead the show methods from the type will be used. I believe that affects for example all the stuff in Images.jl. @timholy do you want to review this PR before we merge it?

[davidanthoff]

Actually, don't merge this. I'll fix and explain later when I have a bit more time.

[davidanthoff]

Ok, here is the problem and how I fixed it: I had the mime savers as the first listed provider now. But Images.jl for example has a show method that can create PNGs that actually might increase or decrease the resolution of the image before formatting it as a PNG file. Clearly that resized image is not what should be written to disc when one calls save. With my original PR here that is what would have happened.

I now just moved the mime writer to the last position, i.e. after the save providers that are listed in the registry right now. I think that should work for everyone: if someone tries to save an image in the Images.jl definition, it should just pick the same routine as previously. If someone tries to save a plot/figure it should just fall through to the mime writer.

So, from my point of view this could be merged. But I think @timholy and @SimonDanisch should take a careful look before merging this. I started looking at this code base a couple of days ago and I don't want to break something in the larger ecosystem that I don't fully understand at this point.

[davidanthoff]

@timholy and @SimonDanisch Oh, and it would be great if we could merge this and then tag a new version in the next couple of days. I'm trying to get all the packages I want to use in my juliacon talk in shape early enough that I can still test things. Totally understand if you are swamped, though.

[SimonDanisch]

Looks good to me! But I haven't done much with mimes yet ;)
But since the tests pass, it's registered as a lower priority saver, I can't see how this can be problematic....

[davidanthoff]

Ok, cool, so maybe we just merge it? I tried this with the Images.jl package and it seemed to work...

[timholy]

Are you sure this won't get triggered in preference to the library-based methods? Can you add a test using a "big" image, e.g., testimage("earth") from TestImages, and make sure that save("myfile.png", img) doesn't save a reduced-size version?

[davidanthoff]

@timholy Hm, this is all tricky to test... I think if a user has a system where the whole Images.jl suite of packages is installed, but not ImageMagick.jl (or the other package that you guys are using on Mac) and then tries to save a picture, it will fall through to my mime writer and save it at a low resolution. Essentially FileIO will see that it has a provider that can save PNG, and will not ask users to install ImageMagick, but instead it will use the mime writer. But if ImageMagick (or the Mac alternative) is installed, I believe FileIO will always try those first. Now, when I tried the TestImages package, it will actually ask me to install ImageMagick because otherwise it can't load the file... So at that point I have ImageMagick installed, and then it will use that to save.

The real question is: can a user system end up in a state where ImageMagick is not installed, but the whole Images.jl family of packages is functional, and then the user tries to save an image? That case will behave differently than before now.

I think the proper solution to this would be to agree on some IOContext properties that can signal to the show method whether it should do the kind of changes that the show method in Images.jl is doing currently. For example, we could have an IOContext property named :full_fidelity, and if that is present and true, any show method for say the PNG mime type should write things without size reductions.

[SimonDanisch]

isn't show actually using ImageMagick?

[davidanthoff]

isn't show actually using ImageMagick?

True... I'm adding the :full_fidelity IOContext property nevertheless (and will open a related PR in Images). I think that can't hurt and seems generally a cleaner design than relying on how the various installations depend on each other.

[davidanthoff]

Ok, I think JuliaImages/Images.jl#642 should make sure we don't run into this problem.

I think at the end of the day this needs to be tested in other packages like Images.jl, i.e. the tests here in FileIO test what can be done without running all the code in the various packages.

[davidanthoff]

@SimonDanisch and @timholy Do you think we can merge this here, in combination with JuliaImages/Images.jl#642? It would be great if FileIO.jl had a release with this change soonish, I'm trying to finish my juliacon talk and have all the pieces in place :)