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

Shapefile custom data #5423

Open
wants to merge 12 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@superDoss

superDoss commented Oct 22, 2018

Hey guys, for a long time now i am using ID editor, and would like to add a feature that in my opinion is missing.

Really not big of a deal, just added support for shp files.

one problem though didn't find a way to test it as i need to read .shp file, and didn't know how to read the files in the testing framework.

Show resolved Hide resolved modules/svg/data.js Outdated
Show resolved Hide resolved modules/svg/data.js Outdated
Show resolved Hide resolved modules/svg/data.js Outdated
@bhousel

This comment has been minimized.

Member

bhousel commented Oct 22, 2018

Thanks @superDoss ! This is pretty great.. I do get asked about supporting shapefiles sometimes.

A few other things I notice are that we will probably need to polyfill a few things for IE and maybe Safari support. (see https://github.com/mbostock/shapefile/blob/master/README.md)

In-browser parsing of dBASE table files requires TextDecoder, part of the Encoding living standard, which is not supported in IE or Safari as of September, 2016. See text-encoding for a browser polyfill.

Also, I don't know how common it is for shapefiles to be in a crs other than EPSG:4326. I'm not sure whether we would want to let people know that iD won't do any reprojection on these. We also don't support the crs property in GeoJSON, so it might not be a big deal.

@Nakaner

This comment has been minimized.

Nakaner commented Oct 23, 2018

Are the geometries just shown as a background layer or are they converted into OSM objects to be uploaded?

If the latter is possible, mentioning the Import Guidelines might be a good idea to avoid unnecessary reverts of buggy imports (not converting attributes into proper tags etc.).

@bhousel

This comment has been minimized.

Member

bhousel commented Oct 23, 2018

Are the geometries just shown as a background layer or are they converted into OSM objects to be uploaded?

Currently just shown

If the latter is possible, mentioning the Import Guidelines might be a good idea to avoid unnecessary reverts of buggy imports (not converting attributes into proper tags etc.).

yep that's an important part of #4633 , which will be coming soon

@superDoss

This comment has been minimized.

superDoss commented Oct 23, 2018

@bhousel i've fixed all of the things you have noted.

I still don't know how to test this as i need to read a shape file, and didn't find a way to do this in phantomjs

@superDoss

This comment has been minimized.

superDoss commented Oct 24, 2018

A few other things I notice are that we will probably need to polyfill a few things for IE and maybe Safari support. (see https://github.com/mbostock/shapefile/blob/master/README.md)

In-browser parsing of dBASE table files requires TextDecoder, part of the Encoding living standard, which is not supported in IE or Safari as of September, 2016. See text-encoding for a browser polyfill.

About that, because of shape file is build from several files (shp,dbf,prj..) didn't figure out the best way to enforce uploading multiple files that are associated, or how to let the user know he needs to upload several files.

So for now there is no problem with dbf encoding as it is not supported and not being parsed.

@quincylvania

This comment has been minimized.

Collaborator

quincylvania commented Oct 24, 2018

didn't figure out the best way to enforce uploading multiple files that are associated

One potential workaround for this is to allow users to upload zip files containing the component files of the shapefile. An important drawback of this is the need to validate the contents and handle many potential errors.

superDoss added some commits Oct 25, 2018

Merge pull request #1 from openstreetmap/master
Add support in shape files for custom-data
@superDoss

This comment has been minimized.

superDoss commented Oct 30, 2018

@bhousel i've just added a working test but for some unknown reason travis have failed to test on node 6, hope you can check it and merge if everything is ok

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment