Skip to content

Fetch according to geo coordinates #40

Merged
goldsmith merged 6 commits intogoldsmith:masterfrom
kazuar:master
Apr 26, 2014
Merged

Fetch according to geo coordinates #40
goldsmith merged 6 commits intogoldsmith:masterfrom
kazuar:master

Conversation

@kazuar
Copy link
Copy Markdown
Contributor

@kazuar kazuar commented Apr 13, 2014

Ability to fetch articles using geo coordinates

Add ability to get coordinates for specific page and also perform
search for pages with geo coordinates
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

add documentation for article_name

@kazuar
Copy link
Copy Markdown
Contributor Author

kazuar commented Apr 15, 2014

  1. Added documentation for article_name
  2. Used article_name in the search parameters instead of checking it with returned results.
  3. Removed all log/print statements
  4. coordinates property returns a tuple of decimals in the format of (lat, lon)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

typo

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

still need a little more insight into the data structures here:

  • can you link to the Mediawiki documentation page on geocordinates in a comment?
  • what's in coordinates[1:]?

also, by "decimal" I meant the python Decimal type, as in:

from decimal import Decimal

one = Decimal("1.00")

Decimal objects are the preferred format for lat/lon over floats and strings because of their precision and methods.

@goldsmith
Copy link
Copy Markdown
Owner

Almost there, thanks for correcting all of the minor details. One bigger thing left: tests! If you check the tests folder, you can see other examples of mocked out HTTP response data to make sure that Wikipedia is still parsing and returning things correctly if we ever decide to refactor.

Looking forward to merging after we have tests for geo coordinates!

@kazuar
Copy link
Copy Markdown
Contributor Author

kazuar commented Apr 19, 2014

  1. Added the unit tests (forgot about these initially :))
  2. The link to the mediawiki is http://www.mediawiki.org/wiki/Extension:GeoData
  3. Doesn't seem to be anything after the first coordinates results (coordinates[1:]). Maybe there are pages with multiple geo coordinates but I'm not sure.
  4. I've changed the coordinates results to be a tuple of Decimals objects (Decimal, Decimal).

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

what's the purpose of doing this instead of just importing decimal and testing if the Decimal objects are equal?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Mostly because I don't really trust decimals.
I can change it if it's an issue.

When do you think that we can merge the new feature?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Sorry, it looks great. This week is pretty busy for me, so I'll merge it in this weekend

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

no worries :).
This is my first contribution so I kinda wanted to make it right.
Thanks and let me know if I can help in any way.

@kazuar
Copy link
Copy Markdown
Contributor Author

kazuar commented Apr 20, 2014

Mostly because I don't really trust decimals.
I can change it if it's an issue.

@goldsmith goldsmith merged commit eeb33f8 into goldsmith:master Apr 26, 2014
@goldsmith
Copy link
Copy Markdown
Owner

I renamed search_loc to geosearch, just FYI. Thanks so much for your work on this pull request! I'll work on pushing v1.2 of wikipedia out to pip.

@kazuar
Copy link
Copy Markdown
Contributor Author

kazuar commented Apr 27, 2014

Thank you.
Let me know if there's anything I can do to help.
It was really fun working with you on this.

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