direct use of WWPlot or TikZImage objects in image()

[Alex-Jordan]

This makes it so if $gr is either a WWPlot or TikZImage object, then you can just do:

image($gr,...)

instead of

image(insertGraph($gr),...)

(This is related to #560.)

[drgrice1]

What about the case of an array of WWPlot or TikZImage objects? I think the check should be done later in the method where an ARRAY ref is checked for, and there is probably a need to take care to check for an immediate ref to a WWPlot or TikZImage object, as well as for WWPlot or TikZImage objects in an ARRAY. Most likely the actual insertGraph call should be inside the while(@image_list) loop.

Edit: After looking at your other related pull request adding the alt attribute, I see that this is related to your comment there about deprecating the array usage.

[Alex-Jordan]

I'm in the middle of your suggestions. I think I will wait on POD until the other PR is resolved. There would be conflicts there.

Perhaps this is not an appropriate place to bring this up, but tikz image production is not working here in two (possibly related) ways. I don't mean because of this change. But I was working with it to test this, and I can't complete the test because of the tikz production not working. And I also wonder if it's moot with the tikz PR that you have open.

(1) If I just do the documented example like:

DOCUMENT();
loadMacros(
  "PGstandard.pl",
  "PGtikz.pl",
);

$image = createTikZImage();
$image->tex(<<END_TIKZ);
\draw (0,0) circle[radius=1.5];
END_TIKZ

BEGIN_TEXT
\{image(insertGraph($image))\}
END_TEXT

ENDDOCUMENT();

There is no image. There is an image URL, but nothing is there. Do I need to look somewhere to make sure write permissions are what they ought to be for creating the image file?

(2) Perhaps related. If I just try to save the result from insertImage($image), I get errors. Like this file:

DOCUMENT();
loadMacros(
  "PGstandard.pl",
  "PGtikz.pl",
);

$image = createTikZImage();
$image->tex(<<END_TIKZ);
\draw (0,0) circle[radius=1.5];
END_TIKZ

$test = insertGraph($image);

ENDDOCUMENT();

with these error messages:

ERRORS from evaluating PG file: 
 'require' trapped by operation mask at /usr/lib/x86_64-linux-gnu/perl/5.26/Encode.pm line 5
   Died within Encode::Alias::find_alias called at line 114 of /usr/lib/x86_64-linux-gnu/perl/5.26/Encode.pm
   from within Encode::getEncoding called at line 132 of /usr/lib/x86_64-linux-gnu/perl/5.26/Encode.pm
   from within Encode::find_encoding called at line 166 of /usr/lib/x86_64-linux-gnu/perl/5.26/Encode.pm
   from within Encode::encode called at line 93 of [WW]/lib/Apache/WeBWorK.pm
   from within TikZImage::draw called at line 717 of [PG]/lib/PGcore.pm
   from within PGcore::insertGraph called at line 568 of [PG]/macros/PG.pl
   from within main::insertGraph called at line 12 of (eval 5146)
Compilation failed in require at /usr/lib/x86_64-linux-gnu/perl/5.26/Encode/Alias.pm line 22

Does anything strike you as familiar or otherwise understood with these issues I am having?

[drgrice1]

Did you make the changes to /etc/ImageMagick-6/policy.xml noted in site.conf.dist?

[drgrice1]

The comment starts on line 117 of that file. This is also on the release notes page.

[drgrice1]

Note that if you are using my pull request #557 (and the defaults with svg) that isn't needed.

[drgrice1]

Everything looks good.

Yeah, resolving merge conflicts is an art of itself.

[drgrice1]

One thing to note is that if my pull request #557 is merged, then this will need to check for the ref eq PGtikz instead (or also as the TikZImage package could still be directly used).

[Alex-Jordan]

I figured it must be something like that. My hunch is I did everything I should have done for the production server moving to 2.16, but failed to review all of the site.conf changes on this development server.

[drdrew42]

Neat! 👍

[pstaabp]

Nice simplifying code.

I did notice that I didn't have to change anything in the policy on image-magick. I seem to be running version 7. Also on my develop machine, it may have a different default policy. I'll have to check when upgrading my production machine.

[Alex-Jordan]

When #559 is resolved, I will rebase this, handle any conflicts, and complete the suggestions here.

[Alex-Jordan]

I updated POD to mention the argument can be a WWPlot or TikZImage. If #557 is resolved before this, I will change "TikZImage" to "PGtikz".

[drgrice1]

I will take a look after dinner.

If this is merged before #557 I will add the package name change there.

[drgrice1]

I tested this with both a single image, and a list of images. It works well.

[drgrice1]

Also, the documentation looks good.

[pstaabp]

If this is merged before #557 I will add the package name change there.

@drgrice1 Should we go ahead with this one and then change #557 as needed?

[taniwallach]

If this is merged before #557 I will add the package name change there.

@drgrice1 Should we go ahead with this one and then change #557 as needed?

Three approvals and positive feedback in the discussion thread. It seems this one is ready to merge first, unless @drgrice1 has some technical reason to sequence in the other order.

I think the release notes should mention the new syntax provide, and make the relevant Wiki page.

[drgrice1]

The sequencing doesn't matter to me.

[pstaabp]

I'm going to go ahead and merge this. We have 3 positive reviews.