-
Notifications
You must be signed in to change notification settings - Fork 131
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
base: master
Are you sure you want to change the base?
Conversation
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
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
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
E.g.: While 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). ISSUE 4
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. |
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. |
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:
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. |
You can see here : #50 |
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: It should read either: 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) But no recycling=x as key... TagInfo also seems to confirm this: I think you are misinterpreting the tag's key/value combinations used in OSM here. In recycling:glass=yes So the entire string recycling:glass is the actual key! This may be confusing... |
The fact is that I tried with highway's tag too.
I tried with other tags, landuse, leisure, barrier, amenity, .... |
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.
|
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] |
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: 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. 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:
|
I've change some line into the code :