Skip to content

tex, string, and separator options for units#1447

Open
Alex-Jordan wants to merge 1 commit into
openwebwork:PG-2.21from
Alex-Jordan:unitsDisplay
Open

tex, string, and separator options for units#1447
Alex-Jordan wants to merge 1 commit into
openwebwork:PG-2.21from
Alex-Jordan:unitsDisplay

Conversation

@Alex-Jordan

Copy link
Copy Markdown
Contributor

This implements one of the suggestions from @dpvc in #1437, and replaces that pull request.

  • Units defined in Units.pm can have string, tex, string_separator, and tex_separator properties.
  • All aliases for a unit will carry over these properties. To demonstrate what to do when this is unwanted, I moved micron to be a named unit instead of an alias.
  • The string property is used when the unit or its aliases are printed as a string.
  • The string property needs to use a string that can actually be printed in hardcopy. There is a comment about this in the file above where the big units hash is defined.
  • Since a unit may be interpolated as part of an argument to Compute(), make sure that any all string properties are also unit names.
  • The tex property is used when the unit or its aliases are printed as tex.
  • The tex_separator property are only used when the unit is not part of a larger fractional unit, and the unit is the first unit following a number. Otherwise a space is used, as has been the case. For example, 180° and 180° s, but 180 s ° (except as tex).
  • The string_separator property is similar, except it is also used even when there is a larger fractional unit. For example, 180°/s.
  • Only deg is using tex_separator and string_separator, with each set to the empty string. I considered doing this with degF and degC, but Google tells me there should be a space for those units for scientific publications.
  • contextUnits.pl was adding aliases for L (liter, liters, litre, and litres). It was cleaner to move those over to Units.pm. If there was a reason they were not already in Units.pm, I could move them back.

One thing that could still be improved, is that for string production, the spaces that are used in between numbers and units (when string_separator is not set), or between units and units should be nonbreaking spaces. I tried things that I though should work, but did not work for one reason or another. So that can be addressed in some future PR.

Try the problem from #1437 for some basic testing. Although you may want to also try with some more complicated units that have unit products and quotients.

@drgrice1 drgrice1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This satisfies my initial request to make the properties centralized and defined in Units.pm, but will not work for what @dpvc asked. That is so that custom units can also take advantage of the new features. For this the addUnit method will need some more modification. Also the copies of the %context::Units::Context::DISPLAY hash made on line 958 for the context::Units::Unit package and line 1442 for the context::Units::NumberWithUnit package are going to be an issue for that as well. If the addUnit method modifies the %context::Units::Context::DISPLAY hash, then the copies will not be modified appropriately. Making those copies is also not particularly efficient. Those should be references instead.

@drgrice1

Copy link
Copy Markdown
Member

Note that something is also needed at line 1856 in the checkMultDiv method of the context::Units::BOP package in order for the string_separator and tex_separator to work for student answers. Perhaps @dpvc can offer suggestions here.

@Alex-Jordan

Copy link
Copy Markdown
Contributor Author

I'll put a pin in this (mark it as draft) until I come back from travel on July 7 (or later). I'll rework it. I can say that I did try at first to create a new %UNITS hash in Units.pm, which later in that same file was boiled down to %known_units to preserve backwards compatibility with things like parserNumberWithUnits.pl. And then I tried modifying contextUnits.pl accordingly to use the more structured %UNITS from Units.pm. That way the data for the new keys is really where it should be the whole time. I just couldn't manage to track all the added complications from the additional structure that this imposed on contextUnits.pl. But I'll try again.

One thing to think about in the meantime. I am leaning to not including the separator keys with this. This was really only included because of the degree symbol when the units are an angle. Not for anything else at present. So, like 60° instead of 60 °. That much would be fine. But what about 60°/s versus 60 °/s? What about if there is a product involving degrees, like 60° s versus 60 ° s? The current attempt makes these come out as 60°/s and 60° s, but more and more I am not sure that is good. So unless the team convinces me otherwise, when I redo this, I will leave out the separator features. If it's preferred, I could leave deg without having tex and string keys, so that deg stays as deg rather than automatically converting to °. Meanwhile authors who directly use ° will continue to see things like 60 °, so it will be a status quo thing, not a regression.

@drgrice1

Copy link
Copy Markdown
Member

I think that the removal of the space for the degree symbol is more important to me than any of the other changes. But do what you can.

I think that 60°/s is correct, not 60 °/s. I am not sure what 60° s is, or even how exactly that would come about.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants