Add stable postcode refs for details lookup and search exclusion#4101
Conversation
- add `PostcodeRef` and parsing helpers - allow postcode refs in excluded IDs - export the new public API type
- resolve postcode refs in postcode-table lookups - accept `place_ref` in the v1 details endpoint - add tests for details and lookup behavior
lonvia
left a comment
lonvia
left a comment
There was a problem hiding this comment.
When I mentioned the spaces in postcode, I did so because I don't think it is a good idea to have spaces in an id, i.e. they should be replaced. Sorry, I should have been more specific. I think replacing with underscore should be fairly safe?
Explains that spaces and hyphens in postcode IDs are replaced with underscores in stable postcode references and adds new examples for clarity. Improves guidance for constructing and using postcode refs in API endpoints.
|
yep.../details now takes postcode=: directly..... The P prefix is only kept for exclude_place_ids (where the entries are mixed with OSM IDs and need disambiguation)... pls check the regx those are ai generated i never quite learned it well. 2 ci test are failing due to SQLAlchemy 2.x incompatibility tough |
|
unrelated to this .. was checking the cicd code to figure out the upper failed job but https://github.com/osm-search/Nominatim/blob/master/.github/workflows/ci-tests.yml#L412 - name: Migrate to master version
run: |
./master/bin/nominatim admin --migrate
./release/bin/nominatim add-data --file Nominatim/test/testdb/additional_api_test.data.osmhere the |
| sa.and_(p.c.osm_id.is_(None), | ||
| p.c.country_code == ref.country_code, | ||
| _normalized_postcode_expr(p.c.postcode) | ||
| == _normalized_postcode_expr(sa.literal(ref.postcode))) |
There was a problem hiding this comment.
Sadly, that won't work because the normalization expression will prevent PostgreSQL from using the index over the postcode column. So, lets just do the conversion space <=> underscore between the postcode reference and the actual saved postcode. I think that should be a 1:1 conversion because there are no underscores in postcodes, so that you can simply do:
p.c.postcode == ref.postcode.replace('_', ' ')
and we are back in business with the index.
Even better if the PostcodeRef does the replacement in its constructor, so that at this point ref already contains a postcode with spaces.
There was a problem hiding this comment.
ok so we can removed the SQL normalization helpers entirely..... and use PostcodeRef.__post_init__ to convert _ to spaces on construction, so lookup/exclusion paths compare directly on postcode without any expression wrapping...am i thinking it correctly ? ya it will indeed be much cleaner
| Collector = Union[LookupCollector, DetailedCollector] | ||
|
|
||
|
|
||
| def _normalized_postcode_expr(expr: sa.ColumnElement[str]) -> sa.ColumnElement[str]: |
There was a problem hiding this comment.
SQLAlchemy types are difficult because they change so much with the different versions. There are a couple of aliases defined in https://github.com/osm-search/Nominatim/blob/master/src/nominatim_api/typing.py. Use those preferably and things should work out with the CI.
There was a problem hiding this comment.
If we go with the SQL normalization approach above.. so these helper funs can be deleted along it... but ya i will keep these aliases in mind
Removes hyphen normalization from postcode handling to match only on spaces, ensuring consistency between API usage, documentation, and database queries.
|
Looks good now, thanks. |
2 participants
Summary
This change adds first-class support for stable postcode references in the API.
Artificial postcode rows do not always have a stable OSM object reference, so this introduces a dedicated ref format:
P<country_code>:<postcode>eg:
Pus:94110Pgb:EH4 7EAcloses #4079
AI usage
The test files rework is done by ai according to my changes
Contributor guidelines (mandatory)