POLAR@3: Add nominatim support#606
Conversation
oeninghe-dataport
requested review from
dopenguin and
warm-coolguy
as code owners
oeninghe-dataport
force-pushed
the
vue3/add-nominatim-support
branch
from
52c8b35 to
4304863
Compare
oeninghe-dataport
changed the title
|
dopenguin
force-pushed
the
vue3/add-nominatim-support
branch
from
4304863 to
599e195
Compare
dopenguin
force-pushed
the
vue3/add-nominatim-support
branch
from
599e195 to
9757271
Compare
|
@oeninghe-dataport Please update the PR. Will review afterwards |
Some changes need to be evaluated after the merges of #447, #542 and #606 as they differ from those PRs and are just added here for type and linting fixes. # Conflicts: # .gitignore # examples/github-io/components/HeroPolarMap.vue # examples/snowbox/YetAnotherEmptyComponent.vue # examples/snowbox/index.js # src/core/components/PolarMap.ce.vue # src/core/types/plugin.ts # src/plugins/reverseGeocoder/store.ts # src/plugins/reverseGeocoder/utils/reverseGeocodeWps.ts # vue2/packages/plugins/Gfi/CHANGELOG.md # vue2/packages/plugins/Gfi/package.json
|
🏓 @dopenguin Updated
|
dopenguin
left a comment
dopenguin
left a comment
There was a problem hiding this comment.
Can we further specify the search so the results are relevant for the client? The default seems ... not helpful :D
| export async function reverseGeocodeWps( | ||
| url: string, | ||
| coordinate: [number, number], | ||
| _epsg: string, |
There was a problem hiding this comment.
I know this is done because the code in src/plugins/reverseGeocoder/store.ts is shorter this way, but I strongly prefer not adding a redundant parameter here.
There was a problem hiding this comment.
I think the information would actually be quite relevant here (though it's unused yet, which has to change).
IIRC, our WPS geocoding only works if the global EPSG and the WPS's EPSG match. We should transform the coordinates here from global EPSG to a configurable WPS-EPSG.
There was a problem hiding this comment.
If it is important, that the maps projection matches the WPS's projection, then please add a separate PR that addresses this issue.
It should be noted that the POLAR nominatim service is currently (due to server constraints) limited to geocoding in Lower Saxony. |
Co-authored-by: Pascal Röhling <[email protected]>
The TODO is moved to #747
Co-authored-by: Pascal Röhling <[email protected]>
dopenguin
left a comment
dopenguin
left a comment
There was a problem hiding this comment.
Can we further specify the search so the results are relevant for the client? The default seems ... not helpful :D
🏓 @oeninghe-dataportIt should be noted that the POLAR nominatim service is currently (due to server constraints) limited to geocoding in Lower Saxony.
Is it possible to update the POLAR nominatim service to include Hamburg as the snowbox typically uses a lot of Hamburg specific services?
|
|
||
| import type { PolarGeoJsonFeature, PolarGeoJsonFeatureCollection } from '@/core' | ||
|
|
||
| import { transformGeometry } from '@/lib/transformGeometry' |
There was a problem hiding this comment.
Will this be the correct import, if #750 is merged or will there be a relative path? Especially as a relative path is used in src/lib/getFeatures/bkg.ts.
In this scenario, I prefer the way it is written in this file.
| geometry: transformGeometry( | ||
| feature.geometry, | ||
| 'EPSG:4326', | ||
| queryParameters.epsg | ||
| ), |
There was a problem hiding this comment.
transformCoordinates includes the information
If there is no available transform between the two projection, the function will throw an error.
So a call with the same projection does not yield an error, correct?
Co-authored-by: Pascal Röhling <[email protected]>
2 participants
Summary
Add support for nominatim in addressSearch and reverseGeocoder
Instructions for local reproduction and review
Snowbox-integrated