Add optional node prefix filter to pagerankGraph (#1090)
Continuing work on #1020. Adding an optional parameter to `nodes()` which enables optional node prefix filtering. Test plan: @decentralion suggested on Discord that the tests should verify: 1) the parameter was passed to `_graph` correctly 2) the augmentation logic was applied correctly The tests I added are identical to the tests in `graph.test`, except that they verify that the result of `pagerankGraph` matches that of `graph`. On one hand, this creates a dependence on `graph`, as these tests don't verify that the filter works correctly, only that graph has applied the filter and returned the iterator. However, my prevailing thought is that it isn't `pagerankGraph's` responsibility to test the behavior of `graph`, and so testing the exact filter results of `pagerankGraph` like we do in `graph.test` isn't the best strategy, and testing that `pagerankGraph`'s results equal `graph`'s results is a better strategy. The tests also check that a `score` is provided alongside each `node` in the iterator, to minimally satisfy @decentralion's second spec. yarn test passes.
This commit is contained in:
parent
17345fcca9
commit
81b7002ce8
|
@ -186,8 +186,8 @@ export class PagerankGraph {
|
|||
return this._syntheticLoopWeight;
|
||||
}
|
||||
|
||||
*_nodesIterator(): Iterator<ScoredNode> {
|
||||
for (const node of this._graph.nodes()) {
|
||||
*_nodesIterator(options?: {|+prefix: NodeAddressT|}): Iterator<ScoredNode> {
|
||||
for (const node of this._graph.nodes(options)) {
|
||||
const score = NullUtil.get(this._scores.get(node));
|
||||
yield {node, score};
|
||||
}
|
||||
|
@ -196,11 +196,13 @@ export class PagerankGraph {
|
|||
/**
|
||||
* Provides node and score for every node in the underlying graph.
|
||||
*
|
||||
* TODO(#1020): Allow optional filtering, as in Graph.nodes.
|
||||
* Optionally, provide a node prefix to return an iterator containing
|
||||
* only node/score objects whose nodes match the provided node prefix.
|
||||
* See Graph.nodes and Address.hasPrefix for details.
|
||||
*/
|
||||
nodes(): Iterator<ScoredNode> {
|
||||
nodes(options?: {|+prefix: NodeAddressT|}): Iterator<ScoredNode> {
|
||||
this._verifyGraphNotModified();
|
||||
return this._nodesIterator();
|
||||
return this._nodesIterator(options);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -1,7 +1,13 @@
|
|||
// @flow
|
||||
|
||||
import sortBy from "lodash.sortby";
|
||||
import {Graph, NodeAddress, EdgeAddress, type Edge} from "./graph";
|
||||
import {
|
||||
Graph,
|
||||
NodeAddress,
|
||||
EdgeAddress,
|
||||
type NodeAddressT,
|
||||
type Edge,
|
||||
} from "./graph";
|
||||
import {PagerankGraph} from "./pagerankGraph";
|
||||
import {advancedGraph} from "./graphTestUtil";
|
||||
import * as NullUtil from "../util/null";
|
||||
|
@ -63,6 +69,57 @@ describe("core/pagerankGraph", () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe("node prefix filter matches graph filter", () => {
|
||||
const n1 = NodeAddress.empty;
|
||||
const n2 = NodeAddress.fromParts(["foo"]);
|
||||
const n3 = NodeAddress.fromParts(["foo", "bar"]);
|
||||
const n4 = NodeAddress.fromParts(["zod", "bar"]);
|
||||
const g = () =>
|
||||
new Graph()
|
||||
.addNode(n1)
|
||||
.addNode(n2)
|
||||
.addNode(n3)
|
||||
.addNode(n4);
|
||||
const pg = () => new PagerankGraph(g(), defaultEvaluator);
|
||||
|
||||
function expectPagerankGraphToEqualGraph(
|
||||
options: {|+prefix: NodeAddressT|} | void
|
||||
) {
|
||||
const pagerankGraphNodes = Array.from(pg().nodes(options)).sort();
|
||||
const graphNodes = Array.from(g().nodes(options)).sort();
|
||||
|
||||
pagerankGraphNodes.forEach(
|
||||
(pgNode, i) =>
|
||||
expect(pgNode.node).toEqual(graphNodes[i]) &&
|
||||
expect(pgNode.score).toBe(0.25)
|
||||
);
|
||||
}
|
||||
|
||||
it("with no options object", () => {
|
||||
expectPagerankGraphToEqualGraph(undefined);
|
||||
});
|
||||
|
||||
it("with prefix filter", () => {
|
||||
expectPagerankGraphToEqualGraph({prefix: n2});
|
||||
});
|
||||
|
||||
it("with empty prefix", () => {
|
||||
expectPagerankGraphToEqualGraph({prefix: NodeAddress.empty});
|
||||
});
|
||||
|
||||
it("with prefix that matches nothing", () => {
|
||||
expectPagerankGraphToEqualGraph({prefix: NodeAddress.fromParts(["2"])});
|
||||
});
|
||||
});
|
||||
|
||||
describe("node prefix filter", () => {
|
||||
it("requires a prefix when options are specified", () => {
|
||||
const pg = new PagerankGraph(nonEmptyGraph(), defaultEvaluator);
|
||||
// $ExpectFlowError
|
||||
expect(() => Array.from(pg.nodes({}))).toThrow("prefix");
|
||||
});
|
||||
});
|
||||
|
||||
describe("edge/edges", () => {
|
||||
it("edges returns the same edges as are in the graph", () => {
|
||||
const g = advancedGraph().graph1();
|
||||
|
|
Loading…
Reference in New Issue