mapbox / shp-write

create and write to shapefiles in pure javascript
BSD 3-Clause "New" or "Revised" License
296 stars 189 forks source link

download() & zip() generate bad shapefiles for polygons or linestrings (write works) #40

Closed glenselle closed 1 year ago

glenselle commented 7 years ago

It appears that download and zip are not properly passing geojson polygons and linestrings to the underlying write function and thus shapefiles created with either of those two helper methods will have a bad geometry when viewed with geospatial software.

To Reproduce To reproduce run the following script w/ node (the geojson is taken straight from the geojson spec site here: http://geojson.org/geojson-spec.html#examples) and import the resulting shapefiles into a geospatial application. The point shapefile will display fine while both the polygon and multiline shapefiles will load but appear invisible or nonexistent. Interestingly, the attribute table will be complete, but zooming to the layer will not show anything. Some GIS applications/software will not even load the shapefiles as they appear to be corrupt.

var shpwrite = require('shp-write')
var fs = require('fs')

var geoJson =  { 
    "type": "FeatureCollection",
    "features": [{ 
        "type": "Feature",
        "geometry": {"type": "Point", "coordinates": [102.0, 0.5]},
        "properties": {"prop0": "value0"}
     }, { 
        "type": "Feature",
        "geometry": {
             "type": "LineString",
             "coordinates": [[102.0, 0.0], [103.0, 1.0], [104.0, 0.0], [105.0, 1.0]]
         },
        "properties": {
            "prop0": "value0",
            "prop1": 0.0
        }
     }, { 
         "type": "Feature",
         "geometry": {
             "type": "Polygon",
             "coordinates": [[[100.0, 0.0], [101.0, 0.0], [101.0, 1.0], [100.0, 1.0], [100.0, 0.0]]]
         },
         "properties": {
             "prop0": "value0",
             "prop1": {"this": "that"}
         }
      }]
    }

fs.writeFileSync('./test.zip', shpwrite.zip(geoJson))

The Issue The issue is related to array nesting in polygons and polylines. If you add another wrapping array around the linestring and polygon coordinate values in the above example, the resulting shapefiles all work as expected. So for the polygon, the coordinate property goes from "coordinates": [[102.0, 0.0], [103.0, 1.0], [104.0, 0.0], [105.0, 1.0]] to "coordinates": [[[102.0, 0.0], [103.0, 1.0], [104.0, 0.0], [105.0, 1.0]]]. Of course, this is not valid geojson. This seems to be a bug since features coordinate arrays should follow the geojson spec.

The Cause/Fix It looks like a commit (5dd750a087de4ed2b888185a2dbfc50e602d68c3) on October 1st, is the cause of these issues. In that commit, the polygon test file is updated by wrapping the points with another array. Of course that obviously breaks compatibility with the geojson spec. If there was a good reason for the changes in that commit, a type check within justType in src/geojson.js seems to resolve the issue. But I don't know this codebase well. I've created a PR with the code that resolves the issue.

drgrubman commented 7 years ago

Just saw this thread - having the same problem for polygons.

glenselle commented 7 years ago

@tmcw It appears we're not the only ones having this problem, and you were the author of the commit which appears to have changed the usage in the last release. I'd really like the fix to be made here...but if nobody looks at this, we're going to have to fork it and fix it ourselves.

tmcw commented 7 years ago

Hi @glenselle - I'd be happy to accept a PR with a fix. I wrote this library, but don't use it on a daily basis, so if you can confirm through daily usage that a fix will work, then that'd be great. Submitting a PR is just as easy as forking, and much better for the ecosystem :)

glenselle commented 7 years ago

@tmcw There's already one made: https://github.com/mapbox/shp-write/pull/41

tmcw commented 7 years ago

Sorry about that - missed it in my notifications. Merged and released as 0.3.1.

sheindel commented 1 year ago

If I followed the conversation above, this should already be resolved. Closing, please re-open if this is relevant and you have a counter example (please see the latest version v0.4.0, to be released soon)

sheindel commented 1 year ago

Good news: v0.4.2 is now available.

Less good news: Mapbox changed their deployment organization so this package is now hosted under a different package

@mapbox/shp-write https://unpkg.com/@mapbox/shp-write@latest/shpwrite.js