Skip to content
This repository has been archived by the owner on Aug 15, 2019. It is now read-only.

Allow tf.loadModel(url) in node.js if fetch exists #1184

Merged
merged 5 commits into from
Jul 25, 2018

Conversation

aman-tiwari
Copy link
Contributor

@aman-tiwari aman-tiwari commented Jul 25, 2018

Description

Check for fetch instead of checking for ENV.get('IS_BROWSER'). This lets people use tf.loadModel('http://....') if they import a fetch polyfill, e.g:

import fetch from 'node-fetch'
global.fetch = fetch

This change is Reviewable

@caisq
Copy link
Collaborator

caisq commented Jul 25, 2018

Thanks for the PR @aman-tiwari . This step is necessary, but not sufficient to get http:// loading working in tfjs-node. I will take a detailed look at that feature soon.

@aman-tiwari
Copy link
Contributor Author

Hey @caisq, not sure what you mean. If one imports the fetch polyfill, this seems to work. One could include the fetch polyfill in tensorflow.js to get it to work without requiring the client to import it, but this seems like the clients responsibility to appropriately prepare the environment.

@aman-tiwari
Copy link
Contributor Author

Added unit tests as per our in-person conversation.

Copy link
Contributor

@nsthorat nsthorat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r1, 1 of 1 files at r3.
Reviewable status: 0 of 1 approvals obtained

@nsthorat nsthorat merged commit 7e6d123 into tensorflow:master Jul 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants