User "Tim Starling" posted a comment on MediaWiki.r81074.
Full URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/81074#c21962
Commit summary:
(bug 235) parser function for conversion of units of measurement.
[[Template:Convert]] on enwiki is a behemoth of a construction that just about
manages to do this sort of conversion, taking {{convert|5|mi|km}} and
outputting "5 miles (8 km)", etc. To port this to another wiki requires
copying over three and a half thousand subtemplates. The additional load
produced by including numerous copies of this template is measurable on large
pages on enwiki, and it eats voraciously into the template limits.
This revision introduces {{#convert: 5 mi | km }}, outputting "8 km" or
thereabouts. See http://www.mediawiki.org/wiki/User:Happy-melon/Convert for
more details, or look at the examples in the parser tests.
In a very rough profile, comparing 50 calls to {{convert}} verses the same 50
calls to the wrapper template shown at the link above, the parser function
implementation reduces page load time by 72%, preprocessor node count by 83%,
post-expand include size by 86% and template argument size by 97%. More
detailed profiling would probably reveal places where extra caching could
improve performance further.
The primary reason for putting it in ParserFunctions instead of its own
extension is availability: PFs are already available across the cluster, and
it's accepted as an essential extension for any wiki wishing to emulate or
mirror WMF content. One less separate extension installed on the cluster is
one less extension which has to be matched by reusers.
It's still missing a lot of units, which I ran out of patience to copy from
{{convert}}; I thought I'd get some feedback on the infrastructure first.
Comment:
Please add doc comments to the functions that lack them.
On line 421:
<pre>
self::$legalDimensions[$var],
self::$legalDimensions[$var2],
</pre>
Presumably this is meant to be $dim and $dim2. I get an "illegal offset type"
warning because $var is an object, with test case <nowiki>{{#convert:1
kn|km/h}}</nowiki>. And with the same test case, on line 728:
<pre>
return $this->prefix
? $this->conversion * self::$prefixes[$this->prefix][0]
: $this->conversion;
</pre>
This is broken with compound units, where $this->prefix is an array.
<nowiki>{{#convert:1 km| mi}}</nowiki> gives "1 mile". The correct answer to
one significant figure would be 0.6 miles, so something is going wrong with the
precision calculations here. Note that when you express a value between 1 and 2
with one significant figure, it's conventional to add another decimal place,
e.g. 1.2 instead of 1.
I'm sorry to say, but these kinds of errors suggest that this module has not
been sufficiently well-tested to justify immediate deployment to Wikimedia. I
think registration of the #convert parser function should be made optional, and
disabled by default for now, unless a lot of extra work is done.
_______________________________________________
MediaWiki-CodeReview mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
