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

Update download_using_xapi.py #82

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

tonyemery
Copy link

I've change some line into the code :

  • Important : Correction of problems adding str() function to get the values.
  • Adding comments (english and french)
  • Adding Messages and prints

@mboeringa
Copy link

Tony,

Are you absolutely sure that this actually fixes the real problems?

I have been running some tests of my own, and to me, it seems the problem is not so much in the conversion of variables (GetParameterAsText is already supposed to convert to string), since I can get results from the tool. However, personally, I see a number of minor issues that need fixing:

ISSUE 1

  • The URL set as the parameter default of the Download URL parameter of the script needs changing, it no longer points to a valid server mirror. This is a actually quite a big issue, since many inexperienced people are probably unaware of what the XAPI is, and what possible valid servers are. So it should point to a valid one.
    The default URL set on the "Download URL" parameter, is pointing to a XAPI server that is no longer available (http://jxapi.openstreetmap.org/xapi/api/0.6/). When I repoint the URL to a server that is available, I can download data without problems.

If you look at this Wiki webpage about the OSM XAPI:

http://wiki.openstreetmap.org/wiki/Xapi

You can see the

http://jxapi.openstreetmap.org/xapi/api/0.6/

URL appears to be some personal server which seems to be down.

It may be better to change the default server / download URL to:

http://www.overpass-api.de/api/xapi_meta?

This is the same server that QGIS calls upon when downloading OSM data. The URL is listed in this QGIS module:

QGIS qgosmdownload.cpp on Github

Please note that the URL visible in that module is not the one that should be used for the XAPI calls! It should be the one I listed above, which is listed on the XAPI OSM wiki page.

This URL and the IP address QGIS accesses while downloading, points to a big professional data centre of Hetzner AG in Germany (https://www.hetzner.de/), and thus seems to point to one of the main servers of OSM, and may be a more reliable source, as QGIS is also using it.

ISSUE 2

  • Query syntax explanation for the Request Predicate parameter in the "Help" section of the tool window needs adjusting, as it is confusing. It suggests to add node, way or relation to the predicate string, while in reality, only the predicate itself must be entered. The tool help suggests to use something like:

way[highway=motorway]

while in reality, you must only enter the true predicate:

highway=motorway

If you enter way[highway=motorway], you get a "bad request" respons of the XAPI server, because the tool will also prepend the Download OSM type parameter, causing a double entry of this value (automatically by the tool and by you as the user).

ISSUE 3

  • There is an issue with the combination of a 'Download OSM type' parameter equal to a wildcard * and a 'Request Predicate' parameter using a wildcard, like building=*. The tool completes, but produces an empty *.osm file without content.

E.g.:
'Download OSM type' : *
and
'Request Predicate' : building=*
_fails_.

While
'Download OSM type' : way
and
'Request Predicate' : building=*
_succeeds_.

Unfortunately, the "fail" does not happen because of a "bad request" respons of the XAPI server, it appears to have succeeded, until you realize the created "osm" XML file is empty (e.g. can not be opened with JOSM or Load OSM File tool).
Actually, I am not sure if I consider this a bug in the tool, or a loophole in the XAPI. I think the XAPI should return a "bad request" when receiving such an URL request. Anyway, leaving that discussion aside, it shouldn't be to hard to check for this condition and warn the user. Alternatively, the whole 'Download OSM type' parameter could be dropped, and users forced to use the actual syntax that the Help now falsely suggests you should use, e.g. "way[building=*]". This would remove the ambiguity, and solve both 2) and 3).

ISSUE 4

  • It would be desirable if the tool converted any projected coordinates to decimal degrees. Currently, if for example the data frame's projected coordinate system is set to WGS 1984 Web Mercator (auxiliary sphere), the same projection most web maps use, than the tool will not fill in the correct decimal degree coordinates for the request extent. QGIS does this. It would be desirable to have the same here. Of course, there is no 100% match between a projected extent and a derived geographic extent, but it could give a close approximation.

Lastly, I am also not to sure about adding French comments in the code, it causes many detected differences in the code, that aren't actual code differences. And how many other languages would you like to see in the code yourself, probably none...? I would stick to English as the sole language for adding code comments.

@tonyemery
Copy link
Author

I tried with many variables, modifying the url servers (I use http://api.openstreetmap.fr/xapi? because I know it works), with and without braquets. Finally, none of my tests have worked. That's why I changed the script by adding comments and messages to see where the script was failing.
Finally, adding the str () functions when retrieving variables, the problem has disappeared.
I don't know if this is the cause of the problem but, in any event, this modifications solves it.
As I told in my first message. I've made a pdf document to explain what I've done, but how can I diffuse it ?

@mboeringa
Copy link

I tried with many variables

I have seen the code changes and tested myself. But I came to the conclusions above about possible issues. I have successfully downloaded stuff without implementing your suggested changes, but I did need to take care of the stuff I listed:

    • Change URL
    • Use properly formatted Request Predicate, without feature type in it
    • Don't actually combine a wildcard OSM type with a wildcard tag (e.g. building=*)

Did you also do 2 and 3?

Also, could you give at least one example here in the tracker of an exact set of values of the parameters that fail at your side, so I can try replicating it here?

What version of ArcGIS are you on? I am still on ArcGIS 10.2.0, Windows 7.

Note: I am not an ESRI employee... just a user as well like you. So hopefully, some of the ESRI contributors will participate as well and do some testing.

@tonyemery
Copy link
Author

You can see here : #50
In fact, This problem seems to be very hard to replicate and only few users are experiencing this.

@mboeringa
Copy link

Tony,

Thanks for the answer.

However, in the first post of that link, I see exactly the problem I have been describing here as issue 3, you didn't select a feature / Download OSM Type, but did use a wildcard statement for the predicate:

Exécution de : XAPIDownload http://api.openstreetmap.fr/xapi? "4.7 43.98 4.96 44.19" * [recycling=*] D:\SIG\recycling.osm

It is this part of the above statement that is causing the issue and the failure:
*** [recycling=*]**

It should read either:
way[recycling=*]
or
*** [recycling=MATERIAL_BEING_RECYCLED]**

Actually, I see a second problem, because the Wike doesn't seem to list a recycle=x (x=MATERIAL_BEING_RECYCLED) key at all, there is:

amenity=recycling (http://wiki.openstreetmap.org/wiki/Tag:amenity%3Drecycling)
and a diverse range of:
recycling:MATERIAL_BEING_RECYCLED=yes/no

But no recycling=x as key... TagInfo also seems to confirm this:
https://taginfo.openstreetmap.org/search?q=recycling#keys

I think you are misinterpreting the tag's key/value combinations used in OSM here.

In recycling:glass=yes
the KEY is: recycling:glass
and the VALUE is: yes

So the entire string recycling:glass is the actual key! This may be confusing...

@tonyemery
Copy link
Author

The fact is that I tried with highway's tag too.
And I tried all the ways you discribe :

  • [highway=*]
  • [highway=residential]
    way [highway=*]
    way [highway=residential]

I tried with other tags, landuse, leisure, barrier, amenity, ....
It doesn't work.
And when I tried with the script I modified, it works. I don't know why, but it works. May be the "GetParameterAsText" doesn't works in the same way with differents windows version, or because we have a different "local and language" options....

@eggwhites
Copy link
Contributor

Thank you Marco for outlining issues clearly, and Tony for confirming the message logging nature of the updates you made to the script. I tested some of the issues in this thread, and will log corresponding bugs as described below.

  1. Update the default XAPI URL from http://jxapi.openstreetmap.org/xapi/api/0.6/ to http://www.overpass-api.de/api/xapi_meta? - logged as Update the default XAPI URL #85

  2. Update the tool help examples to use "highway=motorway", etc., rather than "way[highway=motorway]" - logged as Update Download OSM Data XAPI Tool help text #86

  3. The combination of a "Download OSM type" parameter equal to a wildcard * and a "Request Predicate" parameter using a wildcard, like building=. For this, I couldn't replicate, the .osm file was created and populated OK. I tested with Download OSM Type set to "" and "[highway=*]"

  4. 'It would be desirable if the tool converted any projected coordinates to decimal degrees' - should be logged as an enhancement - logged as XAPI tool logic for download extent (convert to decimal degrees) #87

@mboeringa
Copy link

My main concern is that this pull request, although it may have served you well, maybe is not the "generic" solution it should be to be merged with the main branch.

Personally, having worked as developer for years, I have never been satisfied with "I don't know why, but it works". This is absolutely not meant as critic in your direction, but just to say that in case there is an issue with code I wrote (I didn't develop this particular code at all), I always want to know the last bit of it why it fails, so as to truly deal with the issue. A workaround is in my personal opinion and developer ethic only a last resort.

By the way, and you have probably realized this going over the code yourself, you do not have to supply the "[" brackets for the predicate. The code deals with that, so you can just enter:

highway=residential

instead of:

[highway=residential]

@mboeringa
Copy link

  1. The combination of a "Download OSM type" parameter equal to a wildcard * and a "Request Predicate" parameter using a wildcard, like building=. For this, I couldn't replicate, the .osm file was created and populated OK. I tested with Download OSM Type set to "" and "[highway=*]"

Sorry, did some more testing, and I think I need to correct myself here:

The option below fails, instead of the other suggestion. Notice the added "way" and brackets:
'Download OSM type' : *
and
'Request Predicate' : way[building=*]
fails with an empty *.osm file.

Another point is that I know from QGIS that there is some limit to the area / extent what the XAPI servers will return, above which you can't download. I think the tool should somehow handle this too. Additionally, a download progress indicator, like in QGIS, would be very nice.
Also, I think it would be wise to not allow "Default" as input extent, as in that case no bounding box is added to the XAPI URL request. The code actually deals with Default by inserting an empty string. I have no idea how an XAPI server handles requests without an extent... A small test I just now ran, seems to indicate a problem, the resulting *.osm file was empty. Of course, in the context of this tool, an empty bounding box for the request, seems not appropriate. Default for this tool also conflicts with the generic description of Default extent in the ArcGIS Help:

Default—The tool you are using will determine the processing extent. All tools have a default extent they calculate from their inputs. This default is rarely documented in the tool reference page but is usually obvious.

A description that is not relevant for this situation, as there are no "inputs". I think the parameter should be turned into Required instead of Optional.

So here are three more "enhancement" requests:

  • Find out if there is a limit to XAPI request, and if so, deal with it in the code.
  • Download progress indicator
  • Don't allow "Default" extent as input, set extent parameter to Required.

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.

3 participants