Skip to content
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

Propose new module (save & read) #562

Closed
DenisCarriere opened this issue Jan 31, 2017 · 7 comments
Closed

Propose new module (save & read) #562

DenisCarriere opened this issue Jan 31, 2017 · 7 comments
Assignees

Comments

@DenisCarriere
Copy link
Member

DenisCarriere commented Jan 31, 2017

I seem to constantly have to write the following functions to read & save GeoJSON files (.geojson).

var truncate = require('@turf/truncate')
function read (filename) {
  return JSON.parse(fs.readFileSync(filename, 'utf8'))
}

function save (filename, features) {
  fs.writeFileSync(filename, JSON.stringify(truncate(features), null, 2))
}

An easy solution was to change the GeoJSON filename extension to (.json), but there's many times where the data comes in as (.geojson) and I need to write this helper function.

Solution

Create a FILE SYSTEM group of functions @turf/file-system (I dunno... just throwing that out there).

For start it would have both read & save functions.

var read = require('@turf/file-system').read
var save = require('@turf/file-system').save

For people who only need fs to read & write GeoJSON this would replace require('fs') for GeoJSON purposes.

var fs =require('@turf/file-system')
fs.save('example.geojson', featureCollection)
@tmcw
Copy link
Collaborator

tmcw commented Jan 31, 2017

👍 This sounds useful, I guess the big question is whether save should, by default, use truncate to limit coords.

@DenisCarriere
Copy link
Member Author

Ideally it would be nice to use truncate by default, reducing the GeoJSON filesize is always a good thing to do.

Potential Issue

  • Need to include a try, catch for the required fs module since these features are only compatible with NodeJS.
  • Handling large GeoJSON can cause memory issues, need to detect "how big" the simple write function can handle. We can implement a more advance writer in the future using fs.createWriteStream.

@morganherlocker
Copy link
Member

Note that we had save and load function many, many moons ago. They were dropped for API design reasons. I'm searching for previous ticket discussions, but here is the save issue:

#36

(I'll try to find the issue with the reasoning for why we dropped these)

@morganherlocker
Copy link
Member

Still digging for old tickets / collecting my thoughts on this one for a more cogent explanation (this all happened 4 years ago), but I'll respectfully sum up the devil's advocate opinion with a brief list and an informative link to a similar issue on another project:

  • levelup dealt with a similar dilemma regarding their writeStream api around the same time we originally did. The API was eventually deprecated because it increased the surface are of the module with a deceptively non-optimal solution that the user really should be forced to think through. Bite the bullet and remove WriteStream completely? Level/levelup#199 (comment)
  • the node fs API provides a quick way to save or load data for the "write a quick script use case" var pts = JSON.parse(fs.readFileSync('./points.json')) and fs.writeFileSync('./points.json', JSON.stringify(pts))
  • A node fs API-alike will only work for a subset of cases. For example, turf is designed to work extremely well in streaming scenarios where datasets are enormous. Many users use turf in browser, in which case these modules would be confusing, and they might request parity for remote sources. These use cases are all 100% valid, but they expand the scope in every direction to infinity. Supporting one use case in the API also creates a strong sense that the other routes are closed off. This reasoning is similar to why we don't include any rendering capabilities in turf - there is no single reasonable solution for every task.
  • serialization & parsing of JSON / reading & writing data from disk are not geospatial or geo-statistic operations.

@DenisCarriere
Copy link
Member Author

@morganherlocker Thanks for sharing,

After some thoughts and consideration, having a file-system module is not a geospatial operation and is also not supported in the browser, so I agree with your 4 year old tickets 👍 This makes a lot of sense why you chose to drop these features.

@DenisCarriere
Copy link
Member Author

Also just noticed Sindresorhus has published two read & write JSON modules.

https://www.npmjs.com/package/load-json-file
https://www.npmjs.com/package/write-json-file

Which could be used for these proposed read & write modules.

@DenisCarriere
Copy link
Member Author

I just used load-json-file & write-json-file in the test.js for @turf/truncate without any issues, those libraries are very easy to use and solves read & write issue I had.

https://github.com/Turfjs/turf/blob/master/packages/turf-truncate/test.js

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

No branches or pull requests

3 participants