-
Notifications
You must be signed in to change notification settings - Fork 17
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
Unit in vector processes #330
Comments
Given the tight timing and the fact that this issue is a blocker for other issues, a simple proposal to have an initial solution that allows to be improved in later stages:
It's not ideal because it implies lat-lon degrees for now, but at least it works, unblocks other issues and is future proof (I think) In a later stage we can add support for a user-chosen unit in some way, e.g.:
|
From my experience the "default distance is the unit of CRS" just leads to a lot of errors made by users. I suggest making the If backends don't want to convert or if there's ambiguity in how the conversion happens, an error can be thrown in case the specified unit doesn't match the CRS unit. |
I agree. And also: |
So if I understand correctly, the "unit = unit of CRS" approach is not a favorable option, even when it's just an initial solution?
Instead of a separate "distance": {"value": 10, "unit": "m"}, We already use that approach in |
I'm not sure whether it's the not favorable option or whether @mkadunc just meant that the user should explicitly specify that he wants to specify the value in the unit of the CRS. So should users always explicitly specify a specific unit or is an option for "unit of srs" that the users explicitly selects fine, too? On the other hand, in date_shift we have a |
"unit_of_crs" would also be acceptable, though not needed IMO (almost all CRSs will have either degree or meter as units, and it shouldn't be too difficult for the user to pick the correct one). I prefer the syntax with an object — Re Edzer's opposition to having 'degree' as a distance unit — not sure what to do when CRS is geographic... Users will treat 'degrees' as just another unit, so it will be very hard to dissuade them, unless we come up with a name for this quantity that isn't "distance" (but e.g. "quasi_euclidean_2_norm_of_coordinate_difference" 😉 ). This is probably not a discussion for this issue, but something that should be addressed elsewhere. |
Quick survey:
|
Vector processes only allow distances in meters.
e.g. in vector_buffer would change from "The distance of the buffer in the unit of the spatial reference system." to "The distance of the buffer in meters." and keep the parameter numerical. Vote: For GeoJSON this may need explicit or implicit reprojection, e.g. through load_geojson or vector_reproject -> #412
|
The decision from #414 (comment) (which we also based on this issue) also applies here. We are strict with the errors and throw an error if not meters. We add a chapter to the implementation guideline that back-ends can diverge from this and still implement more. |
With mostly all vector processes we hit the point where you need to specify a length/distance/... and that needs to be specified in a specific unit. So we have seen different proposals across the PRs:
There are several issues with the options:
I think it's not a good idea to discuss this for every new process. So what's the preferred option that we adopt in general?
PS: In existing (raster) processes we have:
The text was updated successfully, but these errors were encountered: