PG warnings cleanup and minor contextFraction issue

[drdrew42]

This PR replaces #505 -- see link for previous comment thread
I've added one more significant change regarding a circular reference introduced by PGresource.pm

PR fix:

• fixed the tangled repo history from PG warnings cleanup and minor contextFraction issue #505

Fixed PGbasicmacros warning:

• Subroutine TEX redefined at...
• Fixed by removing the 'deprecated' TEX subroutine defined earlier in PGbasicmacros

Fixed PGML warnings:

• Unquoted string "null" may clash with future reserved word at... (in PGML)
• "my" variable %bullet masks earlier declaration in same scope at... (in PGML)

a minor change to contextFraction for consistency:

• method TeX called on a Fraction with denominator 1 will return a number, not a string
• this is an issue for homogenous responses from PG -- e.g. correct_answer_latex_string was not consistently a string

[NEW] Fix circular reference in PGresource:

• parent_alias causes a circular reference within the PG object, resulting in memory leakage
• Scalar::Util::weaken reduces the reference count so that garbage collection will not be prevented

[dpvc]

I like this PR much better (without the white-space issues). I did notice another place where the TeX() method can return a numeric value, and also point out that the string() method also suffers from the same problems as the original TeX() method. That should probably be fixed as well.

[dpvc]

Thanks!

[drgrice1]

This looks much better.

I am not sure why the spaces after the commas were removed on line 1425 (line 1419 after previous changes) of PGbasicmacros.pl, though.

Everything looks good other than that though.

[taniwallach]

This PR seems quite related to changes made in rederly#1 but not all the changes are made consistently in both locations.

[drgrice1]

@taniwallach: The first point to be made here is that the Rederly Github project that you are referring to is not part of WeBWorK. There is collaboration, but we you should not be making this comparison, and we should not be trying to make pull requests here be in sync with pull requests there.

Second, all of the points that you brought up have been discussed in the previous pull request that is now closed.

[taniwallach]

@taniwallach: The first point to be made here is that the Rederly Github project that you are referring to is not part of WeBWorK. There is collaboration, but we you should not be making this comparison, and we should not be trying to make pull requests here be in sync with pull requests there.

This is not a comparison of code from the standalone rederer per-se, but of the version of pg being used in that tool.

As I understand it, the standalone renderer is essentially a contribution from Rederly to the WeBWorK project, having been developed to (a) support their specific needs, and (b) to allow rendering of WW content in a more scalable manner for several other needs of the WW community. If I understand some of the undercurrents, it seems likely that at some point in the future webwork2 may interface to the standalone renderer as a new approach to handling rendering, so that we can stop maintaining several slightly different rendering pathways.

As such - IMHO it is in everyone's interest that the versions of pg should be kept in sync. Various factors lead to some changes being made and put into production on local servers and hosted on forks first (just as some new features from develop may be adopted on some servers even before they are merged into develop and certainly before they are in the master branch).

In particular, the standalone renderer now pulls in pg as a Git submodule:

• see https://github.com/rederly/renderer/blob/master/.gitmodules
• and it is currently pulled in from a fork of pg at https://github.com/rederly/pg
• (earlier versions just used git clone from https://github.com/openwebwork/pg.git inside the Dockerfile, but that was changed in rederly/renderer@bc7e07c to use the submodule approach)

In order to make the cooperation sustainable and manageable in the long run - I think "we" want to encourage keeping the version of pg in sync to the best extent possible. Most of the changes made in that fork have been submitted via various PRs to the develop branch.

Where things seemed out-of-sync - I tried to point it out.

Second, all of the points that you brought up have been discussed in the previous pull request that is now closed.

Sorry - I was working on some testing/development related to the standalone renderer, and when I was looking into something (UTF-8 support related) I realized that the versions of pg had differences. I tried to understand quickly what was being changes and then to check that the changes are getting into the main version of pg. I did not review all the discussion of the prior version of this PR.

If corrections to the changes were made to the "real" pg - it would be best if the relevant people (@drdrew42 ?) made sure to "fix" them also in the Rederly fork - to avoid future trouble due to the differences. I had assumed that @drdrew42 would address the comments added here, and @doombot-exe would look into whether the code changes in rederly#3 were submitted to the main repo or not.

[drdrew42]

The changes in this PR have been through several iterations in response to feedback received. This is partially due to poor branch hygiene, and partially due to suggestions/corrections from the community.

I will absolutely second the recommendation that Rederly keep their fork in line with the main PG branch, particularly when 2.16 (or any new version) is released. But I cannot argue the choice to update their own fork between releases.

Accounting for the discrepancies between forks, it seems that they have pulled in the changes from my original PR, and not this one.

[drgrice1]

It is Rederly's project, and Rederly's responsibility to do what they want with their project. If @drdrew42 or any other webwork contributor would like to contribute to their project, and keep it in sync with webwork that is great.

If the WeBWorK project wants to go to utilizing an approach like they use in their project, then that should be done here in the WeBWorK project. Either it should be integrated into the webwork2 repository or another sub project should be made. It would be a bad idea to have webwork become dependent on some external project that is controlled by a proprietary company, regardless of how good their intentions may be (or seem).

[dlglin]

The standalone renderer was developed by Rederly in collaboration with The WeBWorK Project. I still believe that it is a good direction for WeBWorK to refactor the code so that all places use the same renderer, though I agree with @drgrice1 that referring to the Rederly repository is not the right model. I think the right approach is for TWP to fork the renderer repository within openwebwork. We can continue to keep the two in sync as long as it is mutually beneficial.

[taniwallach]

The standalone renderer was developed by Rederly in collaboration with The WeBWorK Project. I still believe that it is a good direction for WeBWorK to refactor the code so that all places use the same renderer, though I agree with @drgrice1 that referring to the Rederly repository is not the right model. I think the right approach is for TWP to fork the renderer repository within openwebwork. We can continue to keep the two in sync as long as it is mutually beneficial.

@drdrew42 - First an apology. I assumed the PR in the Rederly repo was "yours" but committed/accepted by them, as there was no mention there that they had "cherry-picked" your commits from a PR here. Sorry.

@drgrice1 @dlglin - I suggest we "adopt" the Standalone render, either as a "fork" or by asking that the root repository be transferred to openwebwork after we finish WW 2.16/PG 2.16. Then we can try to sync it with the new release and assume control over the TWP official repo. For now - I think we should focus on the new release, and defer the additional load.

[drgrice1]

I am going to merge this soon.

@Alex-Jordan: This is still pending your approval, so if you have any objections to merging this, please chime in.