-
Notifications
You must be signed in to change notification settings - Fork 117
Remove dist importing #289
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 1 approvals obtained (waiting on @WenheLI)
src/nodejs_kernel_backend_test.ts, line 20 at r1 (raw file):
import * as tf from '@tensorflow/tfjs-core'; import {Tensor5D} from '@tensorflow/tfjs-core'; // tslint:disable-next-line:max-line-length
you c an remove this comment now
src/nodejs_kernel_backend_test.ts, line 23 at r1 (raw file):
import {test_util} from '@tensorflow/tfjs-core'; import {NodeJSKernelBackend} from './nodejs_kernel_backend'; const {expectArraysClose} = test_util;
lets just call test_util.expectArraysClose instead of doing this unpacking
src/io/file_system_test.ts, line 28 at r1 (raw file):
import {NodeFileSystem, nodeFileSystemRouter} from './file_system'; const {expectArraysClose} = test_util;
same here lets use test_util.expectArraysClose below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 6 of 8 files at r1, 2 of 2 files at r2.
Reviewable status: complete! 1 of 1 approvals obtained
Ah looks like those exports don't actually exist... cc @annxingyuan did we start exporting these for WebGPU? |
@nsthorat There is a corresponding PR for the extra exports. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 1 of 1 approvals obtained
src/nodejs_kernel_backend.ts, line 31 at r4 (raw file):
import {TensorMetadata, TFEOpAttr, TFJSBinding} from './tfjs_binding'; type Conv2DInfo = backend_util.Conv2DInfo;
I added this line here rather than adding to each use of Conv2DInfo is to avoid an exceeding of max line length.
DEV This pr is to fix dist importing happens in tfjs-node. A corresponding pr is here, tensorflow/tfjs-node#289
Thanks for this PR! We're waiting on 1.2.8 @tensorflow/tfjs to be published before we can merge this. |
Let's reopen this PR on the monorepo once we move tfjs-node there because we'll have a top-level tslint folder which we can share among every package. 1.2.8 is also a large change for tfjs-node which will take some time for us to do. |
This pr is to remove dist importing and add no-import-from-dist rule.
This change is