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

Add category value to xml output and more JOSM compatibilities fixes #2389

Merged
merged 9 commits into from
Aug 19, 2024

Conversation

simonpoole
Copy link
Contributor

This needs some testing, but likely does the trick.

Fixes #2388

@cicku cicku self-assigned this Jul 9, 2024
@simonpoole simonpoole marked this pull request as draft July 9, 2024 06:29
@simonpoole
Copy link
Contributor Author

simonpoole commented Jul 9, 2024

So after running the output through xmlstarlet, seems as if there are five remaining issues:

  • add xlmns attribute
  • lang attribute missing in description elements
  • ELI uses a country code of "ZZ" in one entry and "XN" in another that JOSM doesn't include as valid
  • JOSM restricts max-zoom to 24 and ELI has one instance of 31 being used (which is naturally nonsense)
  • there are a handful of complaints about a point element turning up unexpectedly (needs investigation)

@simonpoole
Copy link
Contributor Author

simonpoole commented Jul 9, 2024

* [ ]  there are a handful of complaints about a point element turning up unexpectedly (needs investigation)

This is due to JOSM limiting the number of points in a "shape" element to 999, potential solution: simply use a bounding box for these entries (8 as of right now), as we already have the bounding box, this means simply dropping the polygon in these cases.

@simonpoole simonpoole marked this pull request as ready for review July 9, 2024 09:44
@simonpoole simonpoole changed the title Add category value to xml output Add category value to xml output and more JOSM compatibilities fixes Jul 20, 2024
@simonpoole
Copy link
Contributor Author

simonpoole commented Jul 23, 2024

For @grischard the following seem to have polygons with more than 999 vertices

Using BBOX for osmim-imagicode-usgs_250k_ant has 1222 vertices
Using BBOX for gsi.go.jp_airphoto_2020 has 1255 vertices
Using BBOX for bev-inspire-orthofoto has 1323 vertices
Using BBOX for South-Tyrol-DSM_2013 has 1013 vertices
Using BBOX for South-Tyrol-DTM_2013 has 1013 vertices
Using BBOX for USDA-NAIP has 1001 vertices
Using BBOX for san_antonio_river_2016_wms has 5509 vertices
Using BBOX for BDGEx_ctm_multi has 1001 vertices
Using BBOX for SP_Itu_-_Eixos has 2716 vertices

The name element was being set to ELIs id value, which seems to be "wrong".

What is unclear is if it even makes sense to set the id value.
@tsmock
Copy link
Contributor

tsmock commented Jul 24, 2024

[...] the following seem to have polygons with more than 999 vertices
[...]
Using bounding box for SPW2021

There should be 121 vertices for SPW2021 (the only one I spot-checked).

@simonpoole
Copy link
Contributor Author

simonpoole commented Aug 1, 2024

[...] the following seem to have polygons with more than 999 vertices
[...]
Using bounding box for SPW2021

There should be 121 vertices for SPW2021 (the only one I spot-checked).

Sorry, that was a last minute rush before I went on vacation, I've corrected the list now.

@tsmock
Copy link
Contributor

tsmock commented Aug 1, 2024

I hope you had a good vacation. :)

Anyway, it looks like some of those high-vertices count polygons can/should be simplified. But probably not in this PR.
For example, san_antonio_river_2016_wms (5509) loses 3504 vertices with 0.5m max error simplification and 5091 with 1m max error.

@grischard grischard merged commit 4a06c29 into osmlab:gh-pages Aug 19, 2024
1 of 2 checks passed
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.

ELI does not load in latest JOSM
4 participants