Move macros from OPL to PG

[pstaabp]

This PR does a few related things.

1. Adds some directory structure to the PG_ROOT/macros directory.
2. Moves needed macros from the OPL macros directory into the PG_ROOT directories.
3. perltidy the macros directory.
4. Updates the t/load_macros.t test to recursively search the PG_ROOT/macros directory
   for macros instead of the assumed old directory structure.

Also for testing, the following homework set was used. set_with_every_macro.def.txt This set contains at least one problem for every macro with problems in the OPL. There are a few macros with no OPL/Contrib problems.

A number of issues arose while doing this.

• there are a number of errors in the OPL macros that occur when rendering the problem set above. These are errors that occur on the develop branch, but are not being shown (they are in the apache logs). These are to be fixed in this PR.
• There appear to be some macros that are obsolete but are used in OPL problems (listed below).
• Many of the errors seen seem to be functionality that is in other macros and there are redefined subroutine errors upon loads. Problems with these macros need to be adjusted in the OPL. A forthcoming PR to the webwork-problem-libraries branch will contain the deletion of the macros as well as updates to the problems. Note: the timing of this will be important so existing OPLs will still function.

Lastly, to keep the size of this discussion box reasonable, all of the changes are documented in PG_ROOT/macros/README.md. Clearly these can be added to this discussion or some other way of documenting all of these change. Because this involved moving files from one repository to another, git will not track specific changes, just additions and deletions.

This is based on #709 because of the update to the testing infrastructure makes it much easier to do testing with this PR.

Todo:

• refactor some of the macros that have been superseded by more current macros: algebraMacros.pl, unionMacros.pl, PGmiscevaluators.pl, PGanswermacros.pl, PGstringevaluators.pl, PGnumericevaluators.pl
• determine how to handle other macros with subroutine redefined errors: PGcommonFunctions.pl, PGanswermacros.pl, PGbasicmacros.pl, PGauxiliaryFunctions.pl,
• Fix the relatively simple errors in the others (contextLimitedRadicalComplex.pl, contextRationalExponent.pl, contextFiniteSolutionSets.pl, SystemsOfLinearEquationsProblemPCC.pl, SolveLinearEquationPCC.pl, interpMacros.pl, PCCfactor.pl)

[drgrice1]

You claim that this is built on top of #709, but it seems there is more than that in here. Looking at the git log of this branch it seems that this is actually built on top of the pg-overhaul branch in my fork (at least as it was a couple of days ago). That branch is built on top of the migration_to_test2_new branch in my fork. These are branches that I have been working on that I have not put in as pull requests yet and I haven't even mentioned that I was working on them yet. The migration_to_test2_new branch is built on top of a combination of the migration_to_test_2_update and pg-envir-work branches in my fork. The migration_to_test_2_update branch is currently a pull request to the branch for pull request #702. The pg-envir-work branch is #709.

So this pull request is pending two or even three of my upcoming pull requests. Also, as mentioned above, that is as the pg-overhaul branch was a couple of days ago, and is missing some of the changes I have made to that branch.

Now with that said, here are some things that I observed in addition to my comments for specific files.

All of the added files (and a few macros from before) need to have the executable bit removed. find /opt/webwork/pg/macros -name '*.pl' -exec chmod -x {} \; will take care of that.

In addition to the comments I left for the t/macros/load_macros.t file, the file needs to be perltidied.

If we are going to add all of these macros to pg, then in some sense we are making them officially adopted macros. Are we sure that is what is desired with all of them? I think I don't completely agree with that. In particular I am not happy with adding macros that are named for a university like CofIdaho_macros.pl and Dartmouthmacros.pl (and others like that). I have tolerated the unionLists.pl and unionTables.pl macros since they have been there for a long time, but I am not okay with adding more. I would actually rather see those stay in the OPL where they currently reside. If the OPL macros must come here, then they should at least be separated from the officially sanctioned macros in some way and not mixed in with the existing macros in the same directory. I think we need to talk about this more.

Many of these added macros are failing the load_macros.t test. If macros can't load without warnings then they need to at least be fixed before they can be added to pg.

[pstaabp]

You claim that this is built on top of #709, but it seems there is more than that in here. Looking at the git log of this branch it seems that this is actually built on top of the pg-overhaul branch in my fork

I guess I did base this on the pg-overhaul branch. There are only two commits here, so can probably fall back to a different branch and cherry pick these if desired. I was looking at any changes to macro files, and tried to get out in front of those. If we decide to go ahead with this new directory structure, we need to be careful to make sure we're moving the latest version of each macro.

If we are going to add all of these macros to pg, then in some sense we are making them officially adopted macros

I agree. Let's discuss these. There are a few (like you pointed out) that seem to be very university specific. I've been trying to think of a different way of handling these, however since there are problems in the OPL (therefore official) that call these macros, I think by default these are official. They were just in a separate location.

Many of these added macros are failing the load_macros.t test. If macros can't load without warnings then they need to at least be fixed before they can be added to pg.

That's on my todo list and I cited that above. I wanted to get this branch out there so we can discuss. Currently, I have only copied files and updated the load_macros.t script.

[drgrice1]

I think that you should base this off of my migration_to_test2_new branch. That has all of the changes to the tests that you need for this pull request, and you don't need any of the changes from the more aggressive pg-overhaul branch for this pull request. If you interactively rebase this branch onto the migraction_to_test2_new branch and drop the first three commits (these are out of date commits in your branch that have been updated in the other branches and the pg-overhaul branch), then you only need to copy macros/contextTrigDegrees.pl to macros/contexts/contextTrigDegrees.pl and perltidy it to resolve a minor conflict in your commits. The you will have your branch without pg-overhaul.

[drgrice1]

Getting closer.

[drgrice1]

Note the macros/math/SystemsOfLinearEquationsProblemPCC.pl file is failing the test because it depends on the contextFraction.pl macro. So you need to add loadMacros('contextFraction.pl') to the beginning of that file.

[pstaabp]

I'm getting the redefined subroutine errors when rendering a problem in WeBWorK. I've been moving the loadMacros call to inside the _init_XXX subs to ensure that macros are only loaded once.

[drgrice1]

I'm getting the redefined subroutine errors when rendering a problem in WeBWorK. I've been moving the loadMacros call to inside the _init_XXX subs to ensure that macros are only loaded once.

Do NOT do that. If you are getting those warnings it is another problem. It should not be an error to load a macro multiple times, and should also not give any warnings. You are not doing something right. I have tested this, and do not get those warnings.

[pstaabp]

Just opened openwebwork/webwork2#1803 and openwebwork/webwork-open-problem-library#949 as companions to this PR.

[pstaabp]

This is now based on #726 and is ready for testing. Of the problems in the set attached above, problems with the macro unionInclude.pl still have an error, but we have decided to deprecate that macro.

Question before "undrafting" this PR: should we determine which macros should be deprecated and not include those in PG or should that be a separate PR?

[pstaabp]

Recently updated macros that were updated on the develop branch. This includes

• niceTables.pl
• PGbasicmacros.pl
• LiveGraphics3D.pl

[drgrice1]

This generally looks good. However there are a few minor issues. Instead of listing them here, I just put in a pull request to your branch for this pull request with suggested changes to address the issues.

[pstaabp]

Just saw that and merged it.

[drgrice1]

I think this is ready to go now.

[pstaabp]

Moved deprecated macros into PG/macros/deprecated. Ready to test again.

[somiaj]

@pstaabp I just ran across a minor issue in macros/FortLewis/AnswerFormatHelp.pl, though with this change unsure if I should report the issue here (in thought it can be fixed after this move) or to the open-problem-library repo.