search

duckduckgo / zeroclickinfo-goodies

DuckDuckGo Instant Answers based on Perl & JavaScript
https://duckduckhack.com/
Other
979 stars 1.76k forks source link

Acres / kilometers conversion off #733

Closed yegg closed 9 years ago

yegg commented 9 years ago

https://twitter.com/eoisaacs/status/530192508391657475

mwmiller commented 9 years ago

@mintsoft Could you work with @elohmrow to get a fix for this ASAP? Let me know if I can help.

elohmrow commented 9 years ago

Sorry I've been so busy. Would it help to make somebody co maint on C::P ?

sent by a bradDroid - please expect bradVity On Nov 6, 2014 8:53 AM, "Matt Miller" notifications@github.com wrote:

@mintsoft https://github.com/mintsoft Could you work with @elohmrow https://github.com/elohmrow to get a fix for this ASAP? Let me know if I can help.

— Reply to this email directly or view it on GitHub https://github.com/duckduckgo/zeroclickinfo-goodies/issues/733#issuecomment-61981331 .

moollaza commented 9 years ago

@chrisjwilsoncom that's odd, I'm assuming you don't have the dependencies installed? Try running just duckpan query -- does it warn that Conversions wasn't loaded because of missing deps?

moollaza commented 9 years ago

hmm @chrisjwilsoncom I'm unable to reproduce, just pulled Goodies repo, and tried your query. It triggered successfully:

codio_-_duckduckhack

There haven't been any recent changes which would affect this AFAIK.

Try running DDG_BLOCK_TRACE=1 duckpan query Conversions and let me see the output :)

MrChrisW commented 9 years ago

@moollaza Attached in all it's beauty :+1:

image

mwmiller commented 9 years ago

@chrisjwilsoncom Sorry for falling out of the loop on this. The problem is almost certainly that the module on which this depends (Convert::Pluggable) is out of date. Area calculations were just recently added. So the goodie will trigger, but your copy of the library won't know how to do the converison.

Try duckpan installdeps from your goodie directory and see if you get any relief.

MrChrisW commented 9 years ago

@mwmiller No worries I know you're a busy man :stuck_out_tongue:

Well that fixes it Successfully installed Convert-Pluggable-0.024 (upgraded from 0.022) :+1:

Should have worked that out myself! Damn dependencies :frowning:

MrChrisW commented 9 years ago

@mwmiller I'm assuming the issue with Convert Pluggable is that the square kilometer factor is set to 100 using hectare as the base unit it should actually be a factor of 0.01?

Does this sound right to you?

mwmiller commented 9 years ago

@chrisjwilsoncom Yes, I assume it's a fairly simple factor problem. We just need to get it corrected and the new version released.

MrChrisW commented 9 years ago

@mwmiller Great! I've created a pull request on the Convert-Pluggable repo. @elohmrow https://github.com/elohmrow/Convert-Pluggable/pull/6

mwmiller commented 9 years ago

@chrisjwilsoncom Thanks for that. Once the updated version is released, we'll need to update the dist.ini to point to the correct version.

If you are interest doing more hacking on Convert::Pluggable, we have some more open issues to do with rate calculations and combinations of imperial units. Each provides some unique challenges which might make them interesting.

MrChrisW commented 9 years ago

@mwmiller Sure. I'll take a look at the rate calculations as this looks semi-easy. As for the imperial unit conversions... they make me shutter with fear... :grimacing:

mintsoft commented 9 years ago

@mintsoft Could you work with @elohmrow to get a fix for this ASAP? Let me know if I can help.

Sorry @mwmiller I totally missed this in the fray of emails!

mintsoft commented 9 years ago

Sorry I've been so busy. Would it help to make somebody co maint on C::P ?

@elohmrow I'm happy to pick this up if it helps you out

moollaza commented 9 years ago

@mwmiller thanks for the insight, glad it's working now :)

mintsoft commented 9 years ago

Should be closed by https://github.com/duckduckgo/zeroclickinfo-goodies/pull/735