dijs / wiki

Wikipedia Interface for Node.js
MIT License
315 stars 61 forks source link

Does not find coordinates if not located in title of page #36

Closed TedYav closed 7 years ago

TedYav commented 7 years ago

Hi there,

If an article does not have its coordinates listed as a display=title manner, this module will not locate them.

Example page: https://en.wikipedia.org/wiki/Catanzaro

I recently added {{Coord|38|54|N|16|36|E|display=title}} to the top of the article. If you delete this line and try to find the coordinates, even though they are present in the info box, the following will result:

Error Fetching Wikipedia Coordinates for City Catanzaro because TypeError: Cannot read property '0' of undefined (My error message added, but TypeError comes from this function in page.js:

    function coordinates() {
        return (0, _util.api)(apiOptions, {
            prop: 'coordinates',
            titles: raw.title
        }).then(function (res) {
            return res.query.pages[raw.pageid].coordinates[0];
        });
    }

I'm not sure where to make adjustment to have this script search in the info box as well as the page header for coordinates. I'd be glad to make the change if someone could direct me to the relevant scripts.

Thanks!

dijs commented 7 years ago

Hmm... Very strange. It just worked for me.

import wiki from './wiki';

wiki().page('Catanzaro').then(page => {
  page.coordinates().then(console.log);
  // { lat: 38.9, lon: 16.6, primary: '', globe: 'earth' }
});
TedYav commented 7 years ago

Hi there :) Sorry for that—should've found a better example! I fixed that page haha.

Here's a working example:

wiki = require('wikijs').default;
wiki().page('Solok')
  .then(page => page.coordinates())
  .then(coordinates => console.log(coordinates))
  .catch(err => console.log(err));

Which yields:

$ node example_failed_query.js
TypeError: Cannot read property '0' of undefined
    at /Users/teoman/Dropbox/Development/Projects/MapBox/Cities Scanning/node_modules/wikijs/dist/page.js:239:50
    at process._tickCallback (internal/process/next_tick.js:103:7)

I noticed in the categories the page is in the category 'Pages using deprecated coordinates format' --> it seems the page just passes the coordinates to the infobox object and doesn't have a {{coord element with display=title set.

For example, I added this code to the Catanzaro page to make it work: {{Coord|38|54|N|16|36|E|display=title}}

It's quite possible that this is a limitation of the Wikipedia API that it can't find coordinates if they're located in the infobox of an article 😄

dijs commented 7 years ago

Could you please checkout this branch https://github.com/dijs/wiki/pull/37 and test the same example you just gave me. I added the ability to check the info box for "deprecated" coords. The format was a little weird, but I cleaned it up a bit. If you have any better ideas on how we could calculate/format the results, let me know.

TedYav commented 7 years ago

Will do :) I do have some ideas and will post them when I get a chance (next day or two).

TedYav commented 7 years ago

Okay, tested the example, works fine and grabs the coordinates. I think what I might do is add some code to parse the results and return the old values as well lat and lon to make the return format consistent.

Running this code right now:

wiki = require('./dist/wiki.js').default;
wiki().page('Solok')
  .then(page => page.coordinates())
  .then(coordinates => console.log(coordinates))
  .then(() => wiki().page('New York City'))
  .then(page => page.coordinates())
  .then(coordinates => console.log(coordinates))
  .catch(err => console.log(err));

Produces:

{ type: 'deprecated',
  latm: 47,
  lats: 59,
  latNS: 'S',
  longm: 39,
  longs: 58,
  longEW: 'E' }
{ lat: 40.7127, lon: -74.0059, primary: '', globe: 'earth' }

If you're cool with it, I'll add a function to generate lat / lon and PR it if that's cool. Also I realized that some pages with deprecated coordinates have yet another format 🙁 so I can parse that as well now that I see where the relevant code is.

dijs commented 7 years ago

Great, so I will merge this, and you can work off the master branch.

dijs commented 7 years ago

Yep, I would much rather have the same lat/lon object, I just didn't know the math to calculate that.