Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support multiple values for the same OSM tag #123

Merged
merged 1 commit into from
Mar 1, 2018
Merged

Support multiple values for the same OSM tag #123

merged 1 commit into from
Mar 1, 2018

Conversation

ialokim
Copy link
Contributor

@ialokim ialokim commented Feb 12, 2018

I've elaborated this little code change to support a config.json file as follows:

{
    "query": {
        "tags": {
            "public_transport:version": "2",
            "operator":  [
                "Incofer",
                "Alpizar S.A."
            ]
        }
    }
}

This generated the following Overpass query:

relation
  (9.8346,-84.2418,10.0365,-83.8507)
  ["public_transport:version" = "2"]
  ["operator" ~ "Incofer|Alpizar S.A."];
(._;);
out body;

Fixes #110

@pantierra
Copy link
Contributor

pantierra commented Feb 25, 2018

Looks good to me, but haven't run the code myself. Have you tried it with special characters, like Estelí?

@ialokim
Copy link
Contributor Author

ialokim commented Feb 26, 2018

Have you tried it with special characters, like Estelí?

It should work as before, the unicode handling isn't changed by this PR. So yes, special characters should be supported as before.

self.tags += unicode('["' + key + '" = "' + value + '"]')
if isinstance(value, list):
value = '|'.join(value)
self.tags += unicode('["' + key + '" ~ "' + value + '"]')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you switch to ~ ?

It would work with a = here too, and would be safer : we don't want Overpass to match regular expression and get matches that partially fit the values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

...and get matches that partially fit the values.

Well, I've simply followed the suggestion by @jamescr in #110, but you are right, that's indeed a problem.

I'll look into it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've changed it accordingly and added regex anchors (^ and $) to prevent partially matching.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks a lot for noticing it!

@grote grote merged commit 5884c07 into grote:master Mar 1, 2018
@ialokim ialokim deleted the overpass-multiple-tag-values branch March 3, 2018 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants