Gmousse / dataframe-js

No Maintenance Intended
https://gmousse.gitbooks.io/dataframe-js/
MIT License
460 stars 38 forks source link

[BUG] #105

Closed blockems closed 4 years ago

blockems commented 4 years ago

Describe the bug Error loading json files on pc's when using fromJSON and a file. On a PC, and __dirname (as well as process.cwd()) returns a lowercase drive location (eg "c:\home\myproj") which fails file io check routine in ./dataframe-js/lib/io.js.

had to add lowercase "c" to the function (see below).

On a related note; does that mean the function will fail unless it's one "c" drive for a desktop? Shouldn't the check its a drive letter check be more [a-z,A-Z]?

To Reproduce Steps to reproduce the behavior:

  1. On a PC

    const dataframe = require('dataframe-js').DataFrame;

function getNote(dbpath) { const dbnotes = new dataframe.fromJSON('c:/Users/block/Documents/GitHub/Notes/database/notes.json').then(df =>{ df.show(); return df; }); }

  1. Call function
  2. Failes with an error

    (node:16364) UnhandledPromiseRejectionWarning: Error: Protocol not supported.

Expected behavior Load the file

Desktop (please complete the following information):

Fix added a "|| path.startsWith("c")" to addFileProtocol(path) in ./lib/io.js

function addFileProtocol(path) { return path.startsWith("/") || path.startsWith("./") || path.startsWith("C") || path.startsWith("c") ? "file://".concat(path) : path; }

Gmousse commented 4 years ago

Hi thank you for your report.

I will look to fix this.

Gmousse commented 4 years ago

Hi @blockems, sorry for the bug, the implementation was...really bad.

It should be better. You can test it on the development branch hotfix/1.4.4

Tell me.

Gmousse commented 4 years ago

Delivered in 1.4.4

taxhelper commented 2 years ago

Hey @Gmousse ,

Thanks for this library.

Unfortunately, I'm still getting the same issue with this file path on windows.

A windows path like C:/myfolder/file.csv isn't coming through as a valid path as it doesn't have a file:// prefix a Protocol not supported error is thrown.

I've had to update my own code to identity windows os and prefix the file:// myself, before passing to DataFrame.fromCSV (commit for example: https://github.com/taxhelper/topshotactivity/commit/09449f284c4d7789149d254917267e159a052ced)

This is where I'm using something like path.join(__dirnam, file)