Module talk:Convert

This is an old revision of this page, as edited by DePiep (talk | contribs) at 14:52, 21 September 2013 (Maintenance category names: Technical aspects). The present address (URL) is a permanent link to this revision, which may differ significantly from the current revision.

Latest comment: 11 years ago by DePiep in topic Maintenance category names

Notes

This module provides an implementation of {{convert}}. See the documentation.

Usage examples:

  • {{convert/sandboxlua|5.2|m|ftin}} → 5.2 metres (17 ft 1 in)
  • {{convert/sandboxlua|5+3/8|in}}5+38 inches (140 mm)
  • {{convert/sandboxlua|1|yd|2|ft|3|in|hand}} → 1 yard 2 feet 3 inches (15.3 hands)
  • {{convert/sandboxlua|60|x|120|m|ft}} → 60 by 120 metres (200 ft × 390 ft)
  • {{convert/sandboxlua|1|e12BTU/cuft|e9kJ/L|lk=on}} → 1 trillion British thermal units per cubic foot (37×10^9 kJ/L)

The testcases show good agreement between the templates and the module. There are some minor rounding and other differences, and the only significant disagreements appear to be problems in the templates. Substantive differences occur at:

  • energy2: different default output for in.lbf
  • energypervolume: template problems
  • exhaustemission: template problem
  • length2: different micro sign (should be "µ" U+00B5, not "μ" GREEK SMALL LETTER MU U+03BC)
  • mole: different micro sign
  • power: calorie disagreements
  • time: module cannot handle unit horse year (unit not used)
  • torque: template problems and minor differences
  • volume: template hectolitre problems
  • misc: module cannot handle units note, spanner, wrench (units not used)
  • options: template problems and minor differences
  • options2: template problems and minor differences
  • spell: minor differences

The module appears to correctly implement the main features of the template. There are some minor disagreements concerning rounding of results, and some units and options have not yet been implemented. More testing is needed. Johnuniq (talk) 10:52, 26 April 2013 (UTC) Updated 09:56, 19 July 2013 (UTC)Reply

Discussion

Some interesting performance challenges exist. The "NewPP limit report" in the html source for this test page says "Lua time usage: 0.295s". Johnuniq (talk) 09:35, 19 February 2013 (UTC)Reply

presuming that a page that requires data conversion might require many of them, and consideing the sheer size of Module:Convert/data, you might want to look at the mw.loadData documentaion and see if you can use this to improve performance. peace - קיפודנחש (aka kipod) (talk) 16:45, 2 March 2013 (UTC)Reply
Mentioned at http://test2.wikipedia.org/wiki/User:Johnuniq -- WOSlinker (talk) 17:13, 2 March 2013 (UTC)Reply
Thanks, but this page is a little out of date, and I will update it with the info that WOSlinker linked to. My comment about "performance challenges" was written when I had not absorbed the fact that some testcase timeouts were due to use of frame:preprocess. It actually looks as if Module:Convert is reasonably fast, although I haven't yet hammered it. In my sandbox, there are 122 calls of {{convert/sandboxlua}}, and the HTML source shows the Lua time usage is 0.348s (2.8 ms per call). That makes me think that Lua is fast enough even with the current bloated modules. Nevertheless, I'm thinking of some ways I could improve performance and will update the "Plans" section above with info, possibly within a few hours. Johnuniq (talk) 04:08, 3 March 2013 (UTC)Reply

I've removed the plans as they were obsolete. Also, while the testcase pages require a significant time to render, the convert module appears to be sufficiently fast, so there is no reason to look for ways to speed it up. I now have 1689 calls in my sandbox, and the Lua time usage is about 1 millisecond per convert. There is a per-page overhead, and it looks like a small number of converts takes around 40 ms total. Johnuniq (talk) 10:52, 26 April 2013 (UTC)Reply

Status?

I'm curious what the status is here, and when you might hope to be ready for deployment, etc.? Dragons flight (talk) 02:13, 6 June 2013 (UTC)Reply

Yes, I've been wondering that too! I believe there are no significant problems: there are some minor rounding differences between the outputs from the template and the module, and some klunkiness where I'm not happy with the result of some combinations of options, but those same options cause the template to fail, so I do not know of anything significant. However, I've still been doing large refactoring, and I need to wait a week after I decide to stop that before moving towards deployment. The item currently at the top of my todo list is the fact that Unicode minus ('−' U+2212) is accepted with the precision (as in {{convert|123|mi|km|−2}}), and I'm wondering whether to support that. The module accepts Unicode minus and − with the input value, but the function for that does a bunch of other stuff so I would need to replace a simple tonumber(x) with yet another function. I'll also spend a couple of days doing some docs, and see if anything turns up in that process. My guess is that in about a week or two I'll ask for opinions on the next step. Deployment would show other problems, for example, there would probably be quite a few unusual units that I have not implemented. Johnuniq (talk) 03:17, 6 June 2013 (UTC)Reply
Some real-life issues caught up with me, and I haven't done any convert work for the last week and a half. I'm just slowly getting back to it. Johnuniq (talk) 03:36, 21 June 2013 (UTC)Reply
My excuse this week is that I have got into a quagmire trying to adapt the module to work properly on bn: (Bengali Wikipedia; anyone interested may like to follow my links). The unit names and so forth are pretty easy to handle, but lots of the code depends on the numbers using English digits, and it will take some time to work out how best to handle translations. Johnuniq (talk) 03:25, 28 June 2013 (UTC)Reply
It turns out that there is quite a lot of tricky stuff involved. The code should now be ok, but I'm still working with some editors there on getting the unit symbols and names fixed. Johnuniq (talk) 11:16, 5 August 2013 (UTC)Reply

Extra units

Following is a quick test of a new module (Module:Convert/extra) showing how temporary units can be created without significant overhead.

Module:Convert/extra should be transcluded into only those pages which refer to a unit that is not defined in Module:Convert/data. Johnuniq (talk) 11:16, 5 August 2013 (UTC)Reply

I have inserted subst: into the above to freeze the results as they were at the time of this comment. The units were just a temporary experiment (which can be seen in this permalink), and I will remove them soon. Johnuniq (talk) 04:52, 14 August 2013 (UTC)Reply

Error message enhancing

IIC, the default inline error message reads like: Conversion error: Cannot convert "torque" to "energy" (the link is to this talkpage).

Let's compare this with the earlier Module:citation/CS1: |accessdate= requires |url= (help). It links a specific helppage section, and adding the page to (example now on this page).

First, the link to this talkpage is very generic (the same for all errors), and not that helpful (are we supposed to report it, or to fix it?). I suggest it links to a documentation page section, at least as precise as the four categories are. It can also add a link like (talk) to this talkpage indeed.

Second, about that orange color. Note that these messages are inline, warning a reader who most likely does not know what to do with it (there are 100 readers for one knowing editor). Isn't there a less-alarming way of messaging? Put () brackets around it, to separate it from content text maybe? And anyway, it is way too dark as a background color (black or blue text does not contrast enough this way). The same standout effect on a white page is provided by a lighter orange color say #ffcc69, makes easier reading.


Together: (Conversion error (talk): Cannot convert "torque" to "energy") -DePiep (talk) 14:17, 19 September 2013 (UTC)Reply

I hope to give a substantive reply later (RL issues are interfering now), but meanwhile I have dumped a reasonably complete list of all messages that the module can display here. That may be useful while considering what should happen. Johnuniq (talk) 03:03, 20 September 2013 (UTC)Reply
It's later, and I'm not likely to get any quality time to spend on this, so I'll just add it to my long todo list. I like the links produced by CS1, but there's a fair bit of work getting that all organized. I don't even have a good doc page, although I made a start here. I prefer your lighter orange, will do soon if no one offers an alternative suggestion. Johnuniq (talk) 10:43, 20 September 2013 (UTC)Reply

@DePiep: I have implemented as much of your above suggestion as I can at the moment. I would like to include links to a help page as above, and have thought about how to implement that. However, it will have to wait for a decent help page, and a bit of time to do the work. I used Special:ExpandTemplates to capture the following:

  • {{convert/sandboxlua|12|m|kg}} → 12 metres ([convert: unit mismatch])
    <span class="noprint" style="background:#ffcc69;">[[Module talk:Convert|Conversion error]]: Cannot convert "length" to "mass"</span>
  • {{convert/sandboxlua2|12|m|ft|abr=off}} → 12 m (39 ft)[convert: invalid option]
    12 metres (39&nbsp;ft)<span class="noprint" style="background:peachpuff;">[[Module talk:Convert|Conversion warning]]: Ignored invalid option "abr=off"</span>

The second example uses the new template {{convert/sandboxlua2}} which includes "warnings = on" (that parameter has to be in the template, not the convert).

I also implemented WOSlinker's suggestion on my talk so tracking categories are only included in the error or warning if the current page is in a wanted namespace (default "0,10", that is, main and template, per your suggestion). In case you missed it, I'll mention here that I updated a reply on my talk to say that some testing shows that 31 warnings were generated in 7,540 converts scraped from various articles. Johnuniq (talk) 09:54, 21 September 2013 (UTC)Reply

Thanks. The link I propose could point to the type-section (in the input list), when applicable (when it is a unit-issue). Like: Help:Convert#length (and add piped label). But maybe the idea Wikid77 launched the other day might be better (using a superimposed linked text). btw, the message could use a starting space. -DePiep (talk) 10:12, 21 September 2013 (UTC)Reply
That is, the warning could use a leading space. Error messages not. -DePiep (talk) 11:06, 21 September 2013 (UTC)Reply

parameter warnings=true

If I am correct, there is a parameter |warnings= that can be set to |warnings=true (see User:Johnuniq/Using the convert module#Module problems). If set, it should fill Category:Convert invalid option (when an errors occurs).


I cannot check the category filling, because I do not know how to force an "invalid option" error. A hint anyone?

Some tests:

  • {{Convert/sandboxlua|5|km|mi}} → 5 kilometres (3.1 mi)
  • {{Convert/sandboxlua|5|km|mi|warnings=true}} → 5 kilometres (3.1 mi)*
  • {{Convert/sandboxlua|5|km|mi|warnings=false}} → 5 kilometres (3.1 mi)*
  • {{Convert/sandboxlua|5|km|mi|warnings=foo}} → 5 kilometres (3.1 mi)*
  • {{Convert/sandboxlua|5|km|mi|warnings=}} → 5 kilometres (3.1 mi)*
  • {{Convert/sandboxlua|5|km|mi|foo=true}} → 5 kilometres (3.1 mi)*

I find this code confusing, because the parameter name "warnings" is used (checked) twice for apparently opposing values. The logic may be right, but clear it is not.

local function add_warning(parms, mcode, text)
    if warnings then
        if parms.warnings == nil then
            parms.warnings = message({ mcode, text })
        end
    end
end

-DePiep (talk) 15:49, 19 September 2013 (UTC)Reply

I hoped that the comments that are in the code would help:
-- If enabled, add a warning that will be displayed after the convert result.
-- To reduce output noise, only the first warning is displayed.
The warnings variable is set from the configuration:
warnings = boolean(args.warnings) -- true if want warnings for invalid options
The line "if warnings then" tests if warnings are enabled.
The line "if parms.warnings == nil then" means only the first warning is displayed (if there already is a warning, parms.warnings will be set, not nil). That is to reduce clutter that would appear in an article if malformed input was given. As I just commented above, a new list of messages is here, and it includes some warnings. Johnuniq (talk) 03:14, 20 September 2013 (UTC)Reply

I've had time to investigate the above interesting bug where using "warnings=true" causes "true" to be appended to the result. Module:Convert includes the following code:

    -- The wikitext with the final result is set (not shown here).
    wikitext = ...
    -- If any warnings have been generated, they are appended.
    if parms.warnings then
        wikitext = wikitext .. parms.warnings
    end

The problem is that the parms table contains what the user entered, and because the convert contains "warnings=true", the module sets parms.warnings = true. That is then appended, and Lua helpfully converts the Boolean "true" to the string "true". I'll think about making that more robust. Johnuniq (talk) 06:29, 20 September 2013 (UTC)Reply

Don't explain it, code should be self-explaining. Just don't use the same name twice. It is bad programming practice. -DePiep (talk) 14:07, 20 September 2013 (UTC)Reply
That's an interesting issue which has been widely debated. At one time I used to make all my variables unique (check my user name!), mainly because I hated searches that gave false hits, although I was inclined to also agree with the "bad programming practice" sentiment. But then namespaces became fashionable and I had to adapt, and now can't see a problem with similar things having the same name. Johnuniq (talk) 00:24, 21 September 2013 (UTC)Reply
The problem is confusion, as I said. For the reader, it is an extra mental step to keep them apart. That is, if the reader realizes a difference. They are not always mentioned in each others vicinity. (About mental steps: after studying (not reading) I discovered that 'mcode' means message code. I do not see why it would not be like msgcode to be clear a day earlier). -DePiep (talk) 09:17, 21 September 2013 (UTC)Reply
(And prefix 'cvt' as in 'cvt_bad_sigfig' probably means message too). -DePiep (talk) 09:22, 21 September 2013 (UTC)Reply
I hear you, but I will offer a suggestion that applies to most code that I write: search the module to see if "mcode" is used somewhere else where there might be an explanation. I know that's not answering you, but it is a practical suggestion that will help in my code perhaps a quarter of the time. Re "cvt_": I wanted a unique prefix so that all the message codes could be found with a search, and while "msg_" would have fitted that description, I picked "cvt_" because it is very easy to find reasons to use "msg" in a program, and conceivably also "msg_". The "cvt_" is to indicate a convert code (perhaps later I would have found reason for other kinds of codes, and used a different prefix for them). The first usage of "cvt_" in the module explains that it is a string used as a key to get message information. I remember your advice about "don't explain", and am mentioning all this to show my reasoning, not in the hope that you will agree.
I fixed the bug that used to be shown above when "warnings=xxx" is used in a convert. Johnuniq (talk) 10:09, 21 September 2013 (UTC)Reply
Thanks for replying. -DePiep (talk) 12:29, 21 September 2013 (UTC)Reply

boolean

The function boolean is wrong. It should return either "false" or "true", never nil. nil is not a boolean value. The input parameter called "text" can be a numerical value too, so is named wrong. No need to bend meanings. -DePiep (talk) 14:10, 20 September 2013 (UTC)Reply

Option value should be "on" just as other such params have. No need to introduce "yes", "y", "1" as option value. -DePiep (talk) 14:24, 20 September 2013 (UTC)Reply
It's true that my boolean function is self-indulgent as I have had to implement similar stuff on different systems where all the possible "true" values are required, however, it's perfectly valid Lua. The parameter is called "text" because it can only be a string, not numeric (it's not intended to be used from other modules where arbitrary parameters could be passed). Also, it's conventional Lua to not return anything for false, although I do feel unclean after doing that, and my more recent functions have tended to always return a value. I'll try to look around some other templates/modules to gauge what boolean option values are generally used, but we're talking about a pretty minor issue that isn't even used currently. One issue about only regarding "on" as true is error reporting—should the module report an error if the invoking template contains "warnings = yes"? Johnuniq (talk) 00:16, 21 September 2013 (UTC)Reply
I trimmed the boolean function so it accepts only "on" and "yes". Johnuniq (talk) 10:11, 21 September 2013 (UTC)Reply
OK then. Saves testing all these variants. -DePiep (talk) 10:59, 21 September 2013 (UTC)Reply

Before going live

Before going live we need more testing. All options, and all extremes (for example). Also we better test the border issues (like input '0'). -DePiep (talk) 22:35, 19 September 2013 (UTC)Reply

The module code is chaotic. -DePiep (talk) 22:42, 19 September 2013 (UTC)Reply
Yes please. More brutal testing would be good, and please let me know any good or bad findings. I guess you've seen User:Johnuniq/Using the convert module which has links to various tests, as well as a "roadblocks" section. I'll plead guilty to "complex", but I can't agree with "chaotic"! Johnuniq (talk) 10:36, 20 September 2013 (UTC)Reply

Range with a negative

About writing a range of values that contain a negative value. Consider

  • {{convert|-8|-|-3|F|C}} → −8 – −3 °F (−22 – −19 °C)
  • {{convert/sandboxlua|-8|-|-3|F|C}} → −8 – −3 °F (−22 – −19 °C)

The minuses and dashes are formally correct in lua, but make awkward writing/reading. WP:MOSNUM gives this option:
If negative values are involved, an en dash might be confusing. Use words instead: −10 to 10, not −10 – 10.
The example would then look like: −8 to −3 °F (−22 to −19 °C). For ease of reading, 'to' should be used in both ranges, even when only one would have a negative. Ideas anyone? -DePiep (talk) 00:17, 21 September 2013 (UTC)Reply

I agree that the en-dash-minus is ugly and should not be used, however I wanted to emulate the current templates as closely as possible (although there is a slight spacing bug in the current template, as shown above). If "to" is wanted, that parameter can be used:
  • {{convert/sandboxlua|-8|to|-3|F|C}} → −8 to −3 °F (−22 to −19 °C)
Johnuniq (talk) 00:29, 21 September 2013 (UTC)Reply
Fair enough. Feature request, maybe later on. -DePiep (talk) 08:20, 21 September 2013 (UTC)Reply

Maintenance category names

The module uses four categories:

Parent: Category:Convert error categories (not used by module, for category grouping only)

I propose name changes:

  1. Make category names plural by standard WP:CATNAME.
  2. Be more descriptive: "Pages with a convert dimension mismatch". This name is longer and less easy, but it is correct to the letter and is according to most maintenance pages. The short names are only usefull for more experienced editors in the topic (you and me), but are sort of code/jargon. Johnuniq replied earlier here.

Proposed names:

  1. Category:Pages with conversion errors
  2. Category:Pages with conversion dimension mismatches
  3. Category:Pages with unknown conversion units
  4. Category:Pages with invalid conversion options
  5. Parent: Category:Pages with conversion messages
Any comments? -DePiep (talk) 12:16, 21 September 2013 (UTC)Reply
Changed into double plurals ("Pages ... errors") copying the CS1 example Category:Articles with incorrect citation syntax. -DePiep (talk) 12:27, 21 September 2013 (UTC)Reply
See Template_talk:Convert#Edit_request_on_20_September_2013 about how to present the error messages. If we choose the Wikid77 proposal (add superscript link, not the colored inline text), we could use the following corresponding maintenance links;
  1. [conversion error]
  2. [dimension mismatch]
  3. [unknown unit]
  4. no superscript link (or [unknown option])
This way we have a triangular connection between message, category and help page section. -DePiep (talk) 13:24, 21 September 2013 (UTC)Reply

Technical aspects

If we are to use the superscripts, some technical questions arise. Usually, such superscripts are constructed in a template using meta-template {{fix}}. So I created example {{convert dimension mismatch}} for this demo. It has the category name, title, etc: If such an error occurs, the module should: 1. transclude this oldstyle template onto the page; 2. should add the error description to the |title= (will be mousehover-text). 3. Could add a more specific error code somehow for more precise linking |anchor=.
Example: Template:Convert dimension mismatch.
Johnuniq, do you like this way to go? Of course, the code could be handcrafted and hardcoded in the module too. -DePiep (talk) 14:52, 21 September 2013 (UTC)Reply