aplbrain / npyjs

Read numpy .npy files in JavaScript
https://aplbrain.github.io/npyjs/
Apache License 2.0
76 stars 21 forks source link

File failing to load. #9

Open wolfiex opened 4 years ago

wolfiex commented 4 years ago

When trying to open a file I get the following. Tried with both the local and absolute paths as well as with file:/// and without.

TypeError: Only absolute URLs are supported
    at getNodeRequestOption
j6k4m8 commented 4 years ago

Can you share some of the context code around this? Is there more information in the error message?

It looks to me like this is coming from the browser if you're requesting a file that can't be accessed from the browser sandbox.

Irelynx commented 4 years ago

@j6k4m8

Can you share some of the context code around this? Is there more information in the error message?

It looks to me like this is coming from the browser if you're requesting a file that can't be accessed from the browser sandbox.

I have a same error in Node v12 I think it's because of node-fetch dependency

npy = require('npyjs');
n = new npy();
n.load('scene/test.npy').then(console.log);
// same error with: n.load(path.resolve('scene/test.npy'))
/* TypeError: Only absolute URLs are supported
    at getNodeRequestOptions (.\node_modules\node-fetch\lib\index.js:1299:9)
    at .\node_modules\node-fetch\lib\index.js:1404:19
    at new Promise (<anonymous>)
    at fetch (.\node_modules\node-fetch\lib\index.js:1401:9)
    at npyjs.load (.\node_modules\npyjs\index.js:99:16)
    at <anonymous>:1:31
    at <anonymous>:2:3
*/

// if change to something like this:
await n.load('file://' + path.resolve('scene/test.npy')).then(console.log);
// another error appears:
// TypeError: Only HTTP(S) protocols are supported

// workaround:
buf = fs.readFileSync('scene/test.npy');
n.parse(buf.buffer.slice(0, buffer.length));
// work as expected
j6k4m8 commented 4 years ago

Thank you @Irelynx for the super helpful info!

I guess the node-fetch dependency wasn't my best choice. How do you feel about different function calls for local files vs HTTP-requestable resources?

Irelynx commented 4 years ago

Thanks for response, @j6k4m8!

Your suggestion makes sense and good enough, but i though it brokes the usage simplicity.

It seems to me that it makes sense to check the execution environment (node / browser), and then check whether the file exists along the path that was passed as an argument in the file system, and if not, use the node-fetch module.

j6k4m8 commented 4 years ago

That sounds like a good plan to me.

I don't have a ton of time to devote to this in the near future so if either of you are interested in submitting a PR I'd be happy to merge! Otherwise it may need to wait a bit, unfortunately!

dfelsent commented 3 years ago
buf = fs.readFileSync('scene/test.npy');
n.parse(buf.buffer.slice(0, buffer.length));

Unfortunately, I'm getting the error ReferenceError: TextDecoder is not defined when I run the workaround. (Also, I needed to change buffer.length to buf.buffer.length.)

j6k4m8 commented 3 years ago

@dfelsent your issue looks like you could fix it by updating Node to ≥11, as TextDecoder doesn't exist in Node prior to that release.

More generally, I guess my question is what's the best move in order to support both browser-fetched and local fs-loaded files.

loretoparisi commented 3 years ago

@j6k4m8 why not just

loadSync(filename) {
        var res = fs.readFileSync(filename, null);
        var out = this.parse(res.buffer);
        return out;
    }

without slicing?

j6k4m8 commented 3 years ago

@loretoparisi nice! would you be willing to PR?

Part of me wonders if we should completely ignore files here and just provide a parse function that assumes the user has either already read in the file bytes or has a byte array to begin with. That avoids the fs and node-fetch deps, which I think are causing the sadness here.