Mark units that are aliases, and use alias string for uString comparisons so sameUnits will allow plurals, etc. (#1193) #1194
Conversation
…sons so sameUnits will allow plurals, etc.
There was a problem hiding this comment.
This looks fine.
It seems that there is more work to do on adding aliases though, particularly for plural and full word forms. I noticed that things like "gram" and "grams" are not accepted, and there are lots of other units like that. Other forms have been added for some units, but not for many others.
| for ('litre', 'litres', 'liter', 'liters') { | ||
| $UNITS{$_} = $UNITS{L}; | ||
| $ALIAS{$_} = 'L'; | ||
| } |
There was a problem hiding this comment.
Why are these extras added here instead of to the Units.pm file? I am curious as to why this is done differently for liters than for all other units that use the aliases key in Units.pm.
There was a problem hiding this comment.
I had originally not wanted to mess with the Units.pm file, so I added them here, mostly as an illustration of how to do that. You can't use aliases here, as the Units.pm file has already processed and removed the aliases properties, so we have to do it by hand here after the fact.
I think the Units.pm file does need a lot of work in terms of aliases, but that wasn't my priority here.
There was a problem hiding this comment.
I see.
I didn't mean to say that you should handle the work on aliases in this pull request. I just wanted to point that out, and that we need to work on it.
Interestingly, I noticed that I get somewhat different behavior from trying to use grams with the parserNumberWithUnits.pl versus with the new contextUnits.pl macro. With the parserNumberWithUnits.pl macro the message is 'gra' is not defined in this context, while with the contextUnits.pl macro the message is 'rams' is not defined in this context. The legacy macro sees ms as a unit and has a problem with gra, but the new macro sees g as a unit and has a problem with rams.
There was a problem hiding this comment.
Yes, the two worked very differently about how they determined the units. The old units context tried to remove units from the end of the student answer and parse the rest as a formula, so it was essentially right-to-left handling of the units. But the new contest integrates units into the parsing as actual MathObjects, so it is done left-to-right, as with all the parsing. When there is a run of characters like "grams", MathObjects will take the maximal substring that matches one of its defined values (constants, functions, etc.), so in this case, since "gr", "gra", "gram", and "grams" are not defined in Units.pm, the g is the longest match.
The units context currently defines squared and square so that you can do square meters or seconds squared. I suppose one could add milli, centi, deci, etc., so that you could use millimeters, decilitres and so on without having to add all the separate aliases.
This PR addresses the issue raised in #1193, where aliases for units are not accepted when
sameUnits => 1is specified.Because the
lib/Units.pmfile removes thealiasesarray from the%Units::known_unitshash, we can't currently determine what is an alias and what isn't, so this PR adds new hashes toUnits.pmthat the units context can use to identify aliases. One maps the original to the array of aliases for it, and one maps from aliases to the original that they are aliases for. The former isn't used in the units context, so can be removed if you want.The changes to the units context itself are to include a new
ustringproperty for units that are aliases of another unit, and changes to theuString()method to cause it to use theustringvalue, if present when constructing the unit string. This is only used internally to compare the units for a correct and student answer, so this doesn't affect the string or TeX value that is produced, but does normalize the unit names for aliases so that any alias will match.