search

dmlc / dgl

Python package built to ease deep learning on graph, on top of existing DL frameworks.
http://dgl.ai
Apache License 2.0
13.47k stars 3.01k forks source link

Undefined names have the potential to raise NameError #290

Closed cclauss closed 5 years ago

cclauss commented 5 years ago

It might be useful to add the flake8 command below to the Jenkins test runs.

The torch, sp, np, and mx issues are probably missing imports...

E901,E999,F821,F822,F823 are the "showstopper" flake8 issues that can halt the runtime with a SyntaxError, NameError, etc. Most other flake8 issues are merely "style violations" -- useful for readability but they do not effect runtime safety.

flake8 testing of https://github.com/dmlc/dgl on Python 3.7.1

$ flake8 . --count --select=E901,E999,F821,F822,F823 --show-source --statistics

./examples/mxnet/gat/gat_batch.py:152:9: F821 undefined name 'torch'
        torch.cuda.set_device(args.gpu)
        ^
./tutorials/models/1_gnn/6_line_graph.py:607:21: F821 undefined name 'sp'
    batched_pmpds = sp.block_diag(pmpds)
                    ^
./tutorials/models/1_gnn/6_line_graph.py:608:22: F821 undefined name 'np'
    batched_labels = np.concatenate(labels, axis=0)
                     ^
./python/dgl/traversal.py:44:11: F821 undefined name '_CAPI_DGLBFSNodes'
    ret = _CAPI_DGLBFSNodes(ghandle, source, reversed)
          ^
./python/dgl/traversal.py:84:11: F821 undefined name '_CAPI_DGLBFSEdges'
    ret = _CAPI_DGLBFSEdges(ghandle, source, reversed)
          ^
./python/dgl/traversal.py:120:11: F821 undefined name '_CAPI_DGLTopologicalNodes'
    ret = _CAPI_DGLTopologicalNodes(ghandle, reversed)
          ^
./python/dgl/traversal.py:165:11: F821 undefined name '_CAPI_DGLDFSEdges'
    ret = _CAPI_DGLDFSEdges(ghandle, source, reversed)
          ^
./python/dgl/traversal.py:236:11: F821 undefined name '_CAPI_DGLDFSLabeledEdges'
    ret = _CAPI_DGLDFSLabeledEdges(
          ^
./python/dgl/graph_index.py:31:9: F821 undefined name '_CAPI_DGLGraphFree'
        _CAPI_DGLGraphFree(self._handle)
        ^
./python/dgl/graph_index.py:41:9: F821 undefined name '_CAPI_DGLGraphAddVertices'
        _CAPI_DGLGraphAddVertices(self._handle, num);
        ^
./python/dgl/graph_index.py:54:9: F821 undefined name '_CAPI_DGLGraphAddEdge'
        _CAPI_DGLGraphAddEdge(self._handle, u, v);
        ^
./python/dgl/graph_index.py:69:9: F821 undefined name '_CAPI_DGLGraphAddEdges'
        _CAPI_DGLGraphAddEdges(self._handle, u_array, v_array)
        ^
./python/dgl/graph_index.py:74:9: F821 undefined name '_CAPI_DGLGraphClear'
        _CAPI_DGLGraphClear(self._handle)
        ^
./python/dgl/graph_index.py:85:21: F821 undefined name '_CAPI_DGLGraphIsMultigraph'
        return bool(_CAPI_DGLGraphIsMultigraph(self._handle))
                    ^
./python/dgl/graph_index.py:105:16: F821 undefined name '_CAPI_DGLGraphNumVertices'
        return _CAPI_DGLGraphNumVertices(self._handle)
               ^
./python/dgl/graph_index.py:115:16: F821 undefined name '_CAPI_DGLGraphNumEdges'
        return _CAPI_DGLGraphNumEdges(self._handle)
               ^
./python/dgl/graph_index.py:130:21: F821 undefined name '_CAPI_DGLGraphHasVertex'
        return bool(_CAPI_DGLGraphHasVertex(self._handle, vid))
                    ^
./python/dgl/graph_index.py:146:30: F821 undefined name '_CAPI_DGLGraphHasVertices'
        return utils.toindex(_CAPI_DGLGraphHasVertices(self._handle, vid_array))
                             ^
./python/dgl/graph_index.py:163:21: F821 undefined name '_CAPI_DGLGraphHasEdgeBetween'
        return bool(_CAPI_DGLGraphHasEdgeBetween(self._handle, u, v))
                    ^
./python/dgl/graph_index.py:182:30: F821 undefined name '_CAPI_DGLGraphHasEdgesBetween'
        return utils.toindex(_CAPI_DGLGraphHasEdgesBetween(self._handle, u_array, v_array))
                             ^
./python/dgl/graph_index.py:199:30: F821 undefined name '_CAPI_DGLGraphPredecessors'
        return utils.toindex(_CAPI_DGLGraphPredecessors(self._handle, v, radius))
                             ^
./python/dgl/graph_index.py:216:30: F821 undefined name '_CAPI_DGLGraphSuccessors'
        return utils.toindex(_CAPI_DGLGraphSuccessors(self._handle, v, radius))
                             ^
./python/dgl/graph_index.py:233:30: F821 undefined name '_CAPI_DGLGraphEdgeId'
        return utils.toindex(_CAPI_DGLGraphEdgeId(self._handle, u, v))
                             ^
./python/dgl/graph_index.py:256:22: F821 undefined name '_CAPI_DGLGraphEdgeIds'
        edge_array = _CAPI_DGLGraphEdgeIds(self._handle, u_array, v_array)
                     ^
./python/dgl/graph_index.py:282:22: F821 undefined name '_CAPI_DGLGraphFindEdges'
        edge_array = _CAPI_DGLGraphFindEdges(self._handle, eid_array)
                     ^
./python/dgl/graph_index.py:308:26: F821 undefined name '_CAPI_DGLGraphInEdges_1'
            edge_array = _CAPI_DGLGraphInEdges_1(self._handle, v[0])
                         ^
./python/dgl/graph_index.py:311:26: F821 undefined name '_CAPI_DGLGraphInEdges_2'
            edge_array = _CAPI_DGLGraphInEdges_2(self._handle, v_array)
                         ^
./python/dgl/graph_index.py:335:26: F821 undefined name '_CAPI_DGLGraphOutEdges_1'
            edge_array = _CAPI_DGLGraphOutEdges_1(self._handle, v[0])
                         ^
./python/dgl/graph_index.py:338:26: F821 undefined name '_CAPI_DGLGraphOutEdges_2'
            edge_array = _CAPI_DGLGraphOutEdges_2(self._handle, v_array)
                         ^
./python/dgl/graph_index.py:363:26: F821 undefined name '_CAPI_DGLGraphEdges'
            edge_array = _CAPI_DGLGraphEdges(self._handle, sorted)
                         ^
./python/dgl/graph_index.py:383:16: F821 undefined name '_CAPI_DGLGraphInDegree'
        return _CAPI_DGLGraphInDegree(self._handle, v)
               ^
./python/dgl/graph_index.py:399:30: F821 undefined name '_CAPI_DGLGraphInDegrees'
        return utils.toindex(_CAPI_DGLGraphInDegrees(self._handle, v_array))
                             ^
./python/dgl/graph_index.py:414:16: F821 undefined name '_CAPI_DGLGraphOutDegree'
        return _CAPI_DGLGraphOutDegree(self._handle, v)
               ^
./python/dgl/graph_index.py:430:30: F821 undefined name '_CAPI_DGLGraphOutDegrees'
        return utils.toindex(_CAPI_DGLGraphOutDegrees(self._handle, v_array))
                             ^
./python/dgl/graph_index.py:446:15: F821 undefined name '_CAPI_DGLGraphVertexSubgraph'
        rst = _CAPI_DGLGraphVertexSubgraph(self._handle, v_array)
              ^
./python/dgl/graph_index.py:482:15: F821 undefined name '_CAPI_DGLGraphEdgeSubgraph'
        rst = _CAPI_DGLGraphEdgeSubgraph(self._handle, e_array)
              ^
./python/dgl/graph_index.py:714:18: F821 undefined name '_CAPI_DGLGraphLineGraph'
        handle = _CAPI_DGLGraphLineGraph(self._handle, backtracking)
                 ^
./python/dgl/graph_index.py:730:24: F821 undefined name '_CAPI_DGLGraphCreate'
        self._handle = _CAPI_DGLGraphCreate(multigraph)
                       ^
./python/dgl/graph_index.py:815:26: F821 undefined name '_CAPI_DGLMapSubgraphNID'
    return utils.toindex(_CAPI_DGLMapSubgraphNID(subgraph.induced_nodes.todgltensor(),
                         ^
./python/dgl/graph_index.py:839:14: F821 undefined name '_CAPI_DGLDisjointUnion'
    handle = _CAPI_DGLDisjointUnion(inputs, len(graphs))
             ^
./python/dgl/graph_index.py:863:15: F821 undefined name '_CAPI_DGLDisjointPartitionBySizes'
        rst = _CAPI_DGLDisjointPartitionBySizes(
              ^
./python/dgl/graph_index.py:867:15: F821 undefined name '_CAPI_DGLDisjointPartitionByNum'
        rst = _CAPI_DGLDisjointPartitionByNum(
              ^
./python/dgl/graph_index.py:898:14: F821 undefined name '_CAPI_DGLGraphCreate'
    handle = _CAPI_DGLGraphCreate(multigraph)
             ^
./python/dgl/immutable_graph_index.py:298:15: F821 undefined name '_CAPI_DGLExpandIds'
        dst = _CAPI_DGLExpandIds(v.todgltensor(), off.todgltensor())
              ^
./python/dgl/immutable_graph_index.py:321:15: F821 undefined name '_CAPI_DGLExpandIds'
        src = _CAPI_DGLExpandIds(v.todgltensor(), off.todgltensor())
              ^
./python/dgl/immutable_graph_index.py:650:20: F821 undefined name 'mx'
        edge_ids = mx.nd.arange(0, len(src), step=1, repeat=1, dtype=np.int32)
                   ^
./python/dgl/immutable_graph_index.py:651:15: F821 undefined name 'mx'
        src = mx.nd.array(src, dtype=np.int64)
              ^
./python/dgl/immutable_graph_index.py:652:15: F821 undefined name 'mx'
        dst = mx.nd.array(dst, dtype=np.int64)
              ^
./python/dgl/immutable_graph_index.py:654:18: F821 undefined name 'mx'
        in_csr = mx.nd.sparse.csr_matrix((edge_ids, (dst, src)),
                 ^
./python/dgl/immutable_graph_index.py:656:19: F821 undefined name 'mx'
        out_csr = mx.nd.sparse.csr_matrix((edge_ids, (src, dst)),
                  ^
./python/dgl/_ffi/base.py:22:21: F821 undefined name 'basestring'
    string_types = (basestring,)
                    ^
./python/dgl/_ffi/base.py:23:34: F821 undefined name 'long'
    numeric_types = (float, int, long, np.float32, np.int32)
                                 ^
./python/dgl/runtime/degree_bucketing.py:100:15: F821 undefined name '_CAPI_DGLDegreeBucketing'
    buckets = _CAPI_DGLDegreeBucketing(mids.todgltensor(), dsts.todgltensor(),
              ^
./python/dgl/runtime/degree_bucketing.py:114:15: F821 undefined name '_CAPI_DGLDegreeBucketingForEdges'
    buckets = _CAPI_DGLDegreeBucketingForEdges(dsts.todgltensor())
              ^
./python/dgl/runtime/degree_bucketing.py:130:19: F821 undefined name '_CAPI_DGLDegreeBucketingForFullGraph'
        buckets = _CAPI_DGLDegreeBucketingForFullGraph(graph._handle)
                  ^
./python/dgl/runtime/degree_bucketing.py:132:19: F821 undefined name '_CAPI_DGLDegreeBucketingForRecvNodes'
        buckets = _CAPI_DGLDegreeBucketingForRecvNodes(graph._handle,
                  ^
./tests/pytorch/test_frame.py:72:45: F821 undefined name 'assert_'
    f.set_initializer(lambda shape, dtype : assert_(False))
                                            ^
./tests/pytorch/test_frame.py:164:45: F821 undefined name 'assert_'
    f.set_initializer(lambda shape, dtype : assert_(False))
                                            ^
58    F821 undefined name 'torch'
58
jermainewang commented 5 years ago

Thanks for pointing out the style issue. How does flake8 compare with pylint? Also, some F821 errors are due to the CAPI calls, for example:

./python/dgl/runtime/degree_bucketing.py:100:15: F821 undefined name '_CAPI_DGLDegreeBucketing'
    buckets = _CAPI_DGLDegreeBucketing(mids.todgltensor(), dsts.todgltensor(),

Here, _CAPI_DGLDegreeBucketing is dynamically registered from the c lib.

cclauss commented 5 years ago

These are not style issues. NameErrors have the potential to halt the runtime.

Just add global _CAPI_DGLDegreeBucketing to help linters (and humans) understand what is going on.

flake8 and PyLint are both linters but my experience is that flake8 with these five tests finds a lot of issues that PyLint misses.

jermainewang commented 5 years ago

Hi @cclauss , I put an item in our roadmap issue #302 . We will include this fix in this cycle. Feel free to comment there. Thank you!