From def1fef192c774be1e8fb5c0c448c6f92da48133 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dandelion=20Man=C3=A9?= Date: Thu, 12 Sep 2019 15:12:17 +0200 Subject: [PATCH] Factor TimelineCredParameters into new module (#1375) The `timelineCred.js` file is a bit of a beast. One way to start slimming it down is to pull the parameters into their own file. This is especially helpful as I'm planning a followon PR that will colocate the default parameter values with their declaration. The naming of everything in the `/timeline/` subdirectory is a bit wonky: it reflects that at the time of creation, "Timeline" designated an experimental version of SourceCred. Now, it is becoming canonical, but the cumbersome naming persists. I haven't made any effort to tackle the name debt here. Test plan: `yarn test` passes; since this is merely a code reorganization, this give me great confidence that the change is correct. I also added a few small tests to the new module. Although the behavior in question is already tested, I think setting up test files liberally is a good practice, as the existence of the test file invites the creation of more tests. --- src/analysis/timeline/params.js | 58 ++++++++++++++++++++++++++ src/analysis/timeline/params.test.js | 19 +++++++++ src/analysis/timeline/timelineCred.js | 59 ++++----------------------- 3 files changed, 84 insertions(+), 52 deletions(-) create mode 100644 src/analysis/timeline/params.js create mode 100644 src/analysis/timeline/params.test.js diff --git a/src/analysis/timeline/params.js b/src/analysis/timeline/params.js new file mode 100644 index 0000000..a567a9d --- /dev/null +++ b/src/analysis/timeline/params.js @@ -0,0 +1,58 @@ +// @flow + +import { + type Weights, + type WeightsJSON, + toJSON as weightsToJSON, + fromJSON as weightsFromJSON, +} from "../weights"; + +/** + * Parameters for computing TimelineCred + * + * The parameters are intended to be user-configurable. + */ +export type TimelineCredParameters = {| + // Determines how quickly cred returns to the PageRank seed vector. If alpha + // is high, then cred will tend to "stick" to nodes that are seeded, e.g. + // issues and pull requests. Alpha should be between 0 and 1. + +alpha: number, + // Determines how quickly cred decays. The decay is 1, then cred never + // decays, and old nodes and edges will retain full weight forever. (This + // would result in cred that is highly biased towards old contributions, as + // they would continue earning cred in every timeslice, forever.) If the + // decay is 0, then weights go to zero the first week after their node/edge + // was created. Should be between 0 and 1. + +intervalDecay: number, + // The weights. This determines how much cred is assigned based on different + // node types, how cred flows across various edge types, and can specify + // manual weights directly on individual nodes. See the docs in + // `analysis/weights` for details. + +weights: Weights, +|}; + +export type TimelineCredParametersJSON = {| + +alpha: number, + +intervalDecay: number, + +weights: WeightsJSON, +|}; + +export function paramsToJSON( + p: TimelineCredParameters +): TimelineCredParametersJSON { + return { + alpha: p.alpha, + intervalDecay: p.intervalDecay, + weights: weightsToJSON(p.weights), + }; +} + +export function paramsFromJSON( + p: TimelineCredParametersJSON +): TimelineCredParameters { + return { + alpha: p.alpha, + intervalDecay: p.intervalDecay, + weights: weightsFromJSON(p.weights), + }; +} diff --git a/src/analysis/timeline/params.test.js b/src/analysis/timeline/params.test.js new file mode 100644 index 0000000..bff9a27 --- /dev/null +++ b/src/analysis/timeline/params.test.js @@ -0,0 +1,19 @@ +// @flow + +import {paramsToJSON, paramsFromJSON} from "./params"; +import {defaultWeights} from "../weights"; +import {NodeAddress} from "../../core/graph"; + +describe("analysis/timeline/params", () => { + it("JSON round trip", () => { + const weights = defaultWeights(); + // Ensure it works with non-default weights + weights.nodeManualWeights.set(NodeAddress.empty, 33); + const p = {alpha: 0.5, intervalDecay: 0.5, weights}; + const j = paramsToJSON(p); + const p_ = paramsFromJSON(j); + const j_ = paramsToJSON(p_); + expect(j).toEqual(j_); + expect(p).toEqual(p_); + }); +}); diff --git a/src/analysis/timeline/timelineCred.js b/src/analysis/timeline/timelineCred.js index cd08416..42224d5 100644 --- a/src/analysis/timeline/timelineCred.js +++ b/src/analysis/timeline/timelineCred.js @@ -17,13 +17,14 @@ import { type Node, } from "../../core/graph"; import { - type Weights, - type WeightsJSON, - toJSON as weightsToJSON, - fromJSON as weightsFromJSON, -} from "../weights"; + type TimelineCredParameters, + paramsToJSON, + paramsFromJSON, + type TimelineCredParametersJSON, +} from "./params"; export type {Interval} from "./interval"; +export type {TimelineCredParameters} from "./params"; /** * A Graph Node wrapped with cred information. @@ -37,30 +38,6 @@ export type CredNode = {| +cred: $ReadOnlyArray, |}; -/** - * Parameters for computing TimelineCred - * - * The parameters are intended to be user-configurable. - */ -export type TimelineCredParameters = {| - // Determines how quickly cred returns to the PageRank seed vector. If alpha - // is high, then cred will tend to "stick" to nodes that are seeded, e.g. - // issues and pull requests. Alpha should be between 0 and 1. - +alpha: number, - // Determines how quickly cred decays. The decay is 1, then cred never - // decays, and old nodes and edges will retain full weight forever. (This - // would result in cred that is highly biased towards old contributions, as - // they would continue earning cred in every timeslice, forever.) If the - // decay is 0, then weights go to zero the first week after their node/edge - // was created. Should be between 0 and 1. - +intervalDecay: number, - // The weights. This determines how much cred is assigned based on different - // node types, how cred flows across various edge types, and can specify - // manual weights directly on individual nodes. See the docs in - // `analysis/weights` for details. - +weights: Weights, -|}; - /** * Represents the timeline cred of a graph. This class wraps all the data * needed to analyze and interpet cred (ie. it has the Graph and the cred @@ -287,30 +264,8 @@ const COMPAT_INFO = {type: "sourcecred/timelineCred", version: "0.5.0"}; export opaque type TimelineCredJSON = Compatible<{| +graphJSON: GraphJSON, - +paramsJSON: ParamsJSON, + +paramsJSON: TimelineCredParametersJSON, +pluginsJSON: $ReadOnlyArray, +credJSON: {[string]: $ReadOnlyArray}, +intervalsJSON: $ReadOnlyArray, |}>; - -type ParamsJSON = {| - +alpha: number, - +intervalDecay: number, - +weights: WeightsJSON, -|}; - -function paramsToJSON(p: TimelineCredParameters): ParamsJSON { - return { - alpha: p.alpha, - intervalDecay: p.intervalDecay, - weights: weightsToJSON(p.weights), - }; -} - -function paramsFromJSON(p: ParamsJSON): TimelineCredParameters { - return { - alpha: p.alpha, - intervalDecay: p.intervalDecay, - weights: weightsFromJSON(p.weights), - }; -}