pelias / openstreetmap

Import pipeline for OSM in to Pelias
MIT License
112 stars 72 forks source link

Category does not get mapped as defined in category_map.js #572

Closed tobiasbartsch closed 1 year ago

tobiasbartsch commented 1 year ago

Describe the bug Categories sometimes do not get assigned as defined in category_map.js Here are two examples:

Steps to Reproduce

Expected behavior The correct categories should be mapped to these addresses.

Environment (please complete the following information):

Additional context Is there something that I am fundamentally misunderstanding of how category mapping should work?

Thank you very much for your help!

missinglink commented 1 year ago

Can you please confirm this is a bug in the code rather than an issue with how your environment is configured?

There are unit tests which you can edit and run using the command npm test, I'd be happy to have a look at any failing tests.

https://github.com/pelias/openstreetmap/blob/master/test/stream/category_mapper.js

Is there something that I am fundamentally misunderstanding of how category mapping should work?

What is your understanding? It needs to be configured prior to running the import and should work as illustrated in the tests

tobiasbartsch commented 1 year ago

Hi @missinglink , thank you so much for your quick reply and for helping with this.

I installed Pelias using Docker and did not modify any of its settings -- do you think I need to make changes to the defaults to make category mapping work?

I ran the unit tests by attaching to the pelias/openstreetmap docker container and running npm test -- they all passed, so whatever is happening here is not caught by them. Is there any integration test we could run to determine whether a specific node is imported with correct category mapping?

When you say "it needs to be configured prior to running the import", do you mean I need to change any of the configurations or should they work as defined in https://github.com/pelias/openstreetmap/blob/master/test/stream/category_mapper.js (which they don't seem to do in an integrated system / outside of the unit tests)

For reference, here are the OSM settings Pelias shipped with for the north-america region:

    "openstreetmap": {
      "download": [
        { "sourceURL": "http://download.geofabrik.de/north-america-latest.osm.pbf" }
      ],
      "leveldbpath": "/tmp",
      "datapath": "/data/openstreetmap",
      "import": [{
        "filename": "north-america-latest.osm.pbf"
      }]
    },

and here is the relevant entry from the docker-compose file:

 openstreetmap:
    image: pelias/openstreetmap:master
    container_name: pelias_openstreetmap
    user: "${DOCKER_USER}"
    volumes:
      - "./pelias.json:/code/pelias.json"
      - "${DATA_DIR}:/data"
      - "./blacklist/:/data/blacklist"
missinglink commented 1 year ago

Categories are not shown by default in the geojson response, are you specifying the categories querystring param?

Screenshot_20230425-201425

tobiasbartsch commented 1 year ago

ohh I had no idea. Sorry for filing this as a bug -- it now works. Is this in the docs somewhere? I thought I had read it all...

tobiasbartsch commented 1 year ago

@missinglink do you know whether I can make categories also show up in the results of the reverse endpoint?

missinglink commented 1 year ago

The categories feature was never officially released despite being in the codebase for like 5+ years.

The categories param can be used as a filter such as categories=nightlife or without the value such as above to just enable them in the output.

What I was referring to earlier was that it's possible to customize the category map by telling docker to use a local copy of the mapping script through what's called a bind mapping in docker.

That allows advanced users to create their own custom categories, but like I said it's more advanced.

missinglink commented 1 year ago

related https://github.com/pelias/api/issues/1405

missinglink commented 1 year ago

I don't think it's enabled for reverse and I don't recall why, maybe it should be, I might have mentioned it in the issue above

tobiasbartsch commented 1 year ago

Got it, thank you very much. It would be very useful to have this in reverse (in order to determine what category of places is close to a lat/lng). Closing this now, and might comment on the related issue. Thanks for all your help!