From 06778ea5503243826589c50425eebe3a9bd170f1 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 18 Apr 2019 16:42:17 -0700 Subject: [PATCH] Separate the "configuration" and "evaluation" phases of chart functions Summary: Depends on D20446. Currently, chart functions are both configured through arguments and evaluated through arguments. This sort of conflates things and makes some logic more difficult than it should be. Instead: - Function arguments are used to configure function behavior. For example, `scale(2)` configures a function which does `f(x) => 2 * x`. - Evaluation is now separate, after configuration. We can get rid of "sourceFunction" (which was basically marking one argument as "this is the thing that gets piped in" in a weird magical way) and "canEvaluate()" and "impulse". Sequences of functions are achieved with `compose(u, v, w)`, which configures a function `f(x) => w(v(u(x)))` (note order is left-to right, like piping `x | u | v | w` to produce `y`). The new flow is: - Every chartable function is `compose(...)` at top level, and composes one or more functions. `compose(x)` is longhand for `id(x)`. This just gives us a root/anchor node. - Figure out a domain, through various means. - Ask the function for a list of good input X values in that domain. This lets function chains which include a "fact" with distinct datapoints tell us that we should evaluate those datapoints. - Pipe those X values through the function. - We get Y values out. - Draw those points. Also: - Adds `accumluate()`. - Adds `sum()`, which is now easy to implement. - Adds `compose()`. - All functions can now always evaluate everywhere, they just return `null` if they are not defined at a given X. - Adds repeatable arguments for `compose(f, g, ...)` and `sum(f, g, ...)`. Test Plan: {F6409890} Reviewers: amckinley Reviewed By: amckinley Subscribers: yelirekim Differential Revision: https://secure.phabricator.com/D20454 --- src/__phutil_library_map__.php | 10 +- .../PhabricatorAccumulateChartFunction.php | 81 +++++++++ .../fact/chart/PhabricatorChartDataQuery.php | 44 +++++ .../fact/chart/PhabricatorChartFunction.php | 169 ++++++++---------- .../PhabricatorChartFunctionArgument.php | 20 +-- ...PhabricatorChartFunctionArgumentParser.php | 101 ++++++----- .../chart/PhabricatorComposeChartFunction.php | 73 ++++++++ .../PhabricatorConstantChartFunction.php | 14 +- .../chart/PhabricatorCosChartFunction.php | 19 +- .../chart/PhabricatorFactChartFunction.php | 93 ++++------ .../PhabricatorHigherOrderChartFunction.php | 56 ++++++ .../chart/PhabricatorScaleChartFunction.php | 18 +- .../chart/PhabricatorShiftChartFunction.php | 18 +- .../chart/PhabricatorSinChartFunction.php | 19 +- .../chart/PhabricatorSumChartFunction.php | 40 +++++ .../fact/chart/PhabricatorXChartFunction.php | 20 --- .../PhabricatorFactChartController.php | 98 +++++----- 17 files changed, 574 insertions(+), 319 deletions(-) create mode 100644 src/applications/fact/chart/PhabricatorAccumulateChartFunction.php create mode 100644 src/applications/fact/chart/PhabricatorComposeChartFunction.php create mode 100644 src/applications/fact/chart/PhabricatorHigherOrderChartFunction.php create mode 100644 src/applications/fact/chart/PhabricatorSumChartFunction.php delete mode 100644 src/applications/fact/chart/PhabricatorXChartFunction.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 3e9d57d4c5..e920e92b8d 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2107,6 +2107,7 @@ phutil_register_library_map(array( 'PhabricatorAccessLog' => 'infrastructure/log/PhabricatorAccessLog.php', 'PhabricatorAccessLogConfigOptions' => 'applications/config/option/PhabricatorAccessLogConfigOptions.php', 'PhabricatorAccessibilitySetting' => 'applications/settings/setting/PhabricatorAccessibilitySetting.php', + 'PhabricatorAccumulateChartFunction' => 'applications/fact/chart/PhabricatorAccumulateChartFunction.php', 'PhabricatorActionListView' => 'view/layout/PhabricatorActionListView.php', 'PhabricatorActionView' => 'view/layout/PhabricatorActionView.php', 'PhabricatorActivitySettingsPanel' => 'applications/settings/panel/PhabricatorActivitySettingsPanel.php', @@ -2695,6 +2696,7 @@ phutil_register_library_map(array( 'PhabricatorCommitSearchEngine' => 'applications/audit/query/PhabricatorCommitSearchEngine.php', 'PhabricatorCommitTagsField' => 'applications/repository/customfield/PhabricatorCommitTagsField.php', 'PhabricatorCommonPasswords' => 'applications/auth/constants/PhabricatorCommonPasswords.php', + 'PhabricatorComposeChartFunction' => 'applications/fact/chart/PhabricatorComposeChartFunction.php', 'PhabricatorConduitAPIController' => 'applications/conduit/controller/PhabricatorConduitAPIController.php', 'PhabricatorConduitApplication' => 'applications/conduit/application/PhabricatorConduitApplication.php', 'PhabricatorConduitCallManagementWorkflow' => 'applications/conduit/management/PhabricatorConduitCallManagementWorkflow.php', @@ -3425,6 +3427,7 @@ phutil_register_library_map(array( 'PhabricatorHeraldContentSource' => 'applications/herald/contentsource/PhabricatorHeraldContentSource.php', 'PhabricatorHexdumpDocumentEngine' => 'applications/files/document/PhabricatorHexdumpDocumentEngine.php', 'PhabricatorHighSecurityRequestExceptionHandler' => 'aphront/handler/PhabricatorHighSecurityRequestExceptionHandler.php', + 'PhabricatorHigherOrderChartFunction' => 'applications/fact/chart/PhabricatorHigherOrderChartFunction.php', 'PhabricatorHomeApplication' => 'applications/home/application/PhabricatorHomeApplication.php', 'PhabricatorHomeConstants' => 'applications/home/constants/PhabricatorHomeConstants.php', 'PhabricatorHomeController' => 'applications/home/controller/PhabricatorHomeController.php', @@ -4725,6 +4728,7 @@ phutil_register_library_map(array( 'PhabricatorSubscriptionsUIEventListener' => 'applications/subscriptions/events/PhabricatorSubscriptionsUIEventListener.php', 'PhabricatorSubscriptionsUnsubscribeEmailCommand' => 'applications/subscriptions/command/PhabricatorSubscriptionsUnsubscribeEmailCommand.php', 'PhabricatorSubtypeEditEngineExtension' => 'applications/transactions/engineextension/PhabricatorSubtypeEditEngineExtension.php', + 'PhabricatorSumChartFunction' => 'applications/fact/chart/PhabricatorSumChartFunction.php', 'PhabricatorSupportApplication' => 'applications/support/application/PhabricatorSupportApplication.php', 'PhabricatorSyntaxHighlighter' => 'infrastructure/markup/PhabricatorSyntaxHighlighter.php', 'PhabricatorSyntaxHighlightingConfigOptions' => 'applications/config/option/PhabricatorSyntaxHighlightingConfigOptions.php', @@ -4952,7 +4956,6 @@ phutil_register_library_map(array( 'PhabricatorWorkingCopyDiscoveryTestCase' => 'applications/repository/engine/__tests__/PhabricatorWorkingCopyDiscoveryTestCase.php', 'PhabricatorWorkingCopyPullTestCase' => 'applications/repository/engine/__tests__/PhabricatorWorkingCopyPullTestCase.php', 'PhabricatorWorkingCopyTestCase' => 'applications/repository/engine/__tests__/PhabricatorWorkingCopyTestCase.php', - 'PhabricatorXChartFunction' => 'applications/fact/chart/PhabricatorXChartFunction.php', 'PhabricatorXHPASTDAO' => 'applications/phpast/storage/PhabricatorXHPASTDAO.php', 'PhabricatorXHPASTParseTree' => 'applications/phpast/storage/PhabricatorXHPASTParseTree.php', 'PhabricatorXHPASTViewController' => 'applications/phpast/controller/PhabricatorXHPASTViewController.php', @@ -7987,6 +7990,7 @@ phutil_register_library_map(array( 'PhabricatorAccessLog' => 'Phobject', 'PhabricatorAccessLogConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorAccessibilitySetting' => 'PhabricatorSelectSetting', + 'PhabricatorAccumulateChartFunction' => 'PhabricatorChartFunction', 'PhabricatorActionListView' => 'AphrontTagView', 'PhabricatorActionView' => 'AphrontView', 'PhabricatorActivitySettingsPanel' => 'PhabricatorSettingsPanel', @@ -8691,6 +8695,7 @@ phutil_register_library_map(array( 'PhabricatorCommitSearchEngine' => 'PhabricatorApplicationSearchEngine', 'PhabricatorCommitTagsField' => 'PhabricatorCommitCustomField', 'PhabricatorCommonPasswords' => 'Phobject', + 'PhabricatorComposeChartFunction' => 'PhabricatorHigherOrderChartFunction', 'PhabricatorConduitAPIController' => 'PhabricatorConduitController', 'PhabricatorConduitApplication' => 'PhabricatorApplication', 'PhabricatorConduitCallManagementWorkflow' => 'PhabricatorConduitManagementWorkflow', @@ -9520,6 +9525,7 @@ phutil_register_library_map(array( 'PhabricatorHeraldContentSource' => 'PhabricatorContentSource', 'PhabricatorHexdumpDocumentEngine' => 'PhabricatorDocumentEngine', 'PhabricatorHighSecurityRequestExceptionHandler' => 'PhabricatorRequestExceptionHandler', + 'PhabricatorHigherOrderChartFunction' => 'PhabricatorChartFunction', 'PhabricatorHomeApplication' => 'PhabricatorApplication', 'PhabricatorHomeConstants' => 'PhabricatorHomeController', 'PhabricatorHomeController' => 'PhabricatorController', @@ -11053,6 +11059,7 @@ phutil_register_library_map(array( 'PhabricatorSubscriptionsUIEventListener' => 'PhabricatorEventListener', 'PhabricatorSubscriptionsUnsubscribeEmailCommand' => 'MetaMTAEmailTransactionCommand', 'PhabricatorSubtypeEditEngineExtension' => 'PhabricatorEditEngineExtension', + 'PhabricatorSumChartFunction' => 'PhabricatorHigherOrderChartFunction', 'PhabricatorSupportApplication' => 'PhabricatorApplication', 'PhabricatorSyntaxHighlighter' => 'Phobject', 'PhabricatorSyntaxHighlightingConfigOptions' => 'PhabricatorApplicationConfigOptions', @@ -11323,7 +11330,6 @@ phutil_register_library_map(array( 'PhabricatorWorkingCopyDiscoveryTestCase' => 'PhabricatorWorkingCopyTestCase', 'PhabricatorWorkingCopyPullTestCase' => 'PhabricatorWorkingCopyTestCase', 'PhabricatorWorkingCopyTestCase' => 'PhabricatorTestCase', - 'PhabricatorXChartFunction' => 'PhabricatorChartFunction', 'PhabricatorXHPASTDAO' => 'PhabricatorLiskDAO', 'PhabricatorXHPASTParseTree' => 'PhabricatorXHPASTDAO', 'PhabricatorXHPASTViewController' => 'PhabricatorController', diff --git a/src/applications/fact/chart/PhabricatorAccumulateChartFunction.php b/src/applications/fact/chart/PhabricatorAccumulateChartFunction.php new file mode 100644 index 0000000000..074219504c --- /dev/null +++ b/src/applications/fact/chart/PhabricatorAccumulateChartFunction.php @@ -0,0 +1,81 @@ +newArgument() + ->setName('x') + ->setType('function'), + ); + } + + public function getDomain() { + return $this->getArgument('x')->getDomain(); + } + + public function newInputValues(PhabricatorChartDataQuery $query) { + return $this->getArgument('x')->newInputValues($query); + } + + public function evaluateFunction(array $xv) { + // First, we're going to accumulate the underlying function. Then + // we'll map the inputs through the accumulation. + + $datasource = $this->getArgument('x'); + + // Use an unconstrained query to pull all the data from the underlying + // source. We need to accumulate data since the beginning of time to + // figure out the right Y-intercept -- otherwise, we'll always start at + // "0" wherever our domain begins. + $empty_query = new PhabricatorChartDataQuery(); + + $datasource_xv = $datasource->newInputValues($empty_query); + if (!$datasource_xv) { + // TODO: Maybe this should just be an error? + $datasource_xv = $xv; + } + + $yv = $datasource->evaluateFunction($datasource_xv); + + $map = array_combine($datasource_xv, $yv); + + $accumulator = 0; + foreach ($map as $x => $y) { + $accumulator += $y; + $map[$x] = $accumulator; + } + + // The value of "accumulate(x)" is the largest datapoint in the map which + // is no larger than "x". + + $map_x = array_keys($map); + $idx = -1; + $max = count($map_x) - 1; + + $yv = array(); + + $value = 0; + foreach ($xv as $x) { + // While the next "x" we need to evaluate the function at lies to the + // right of the next datapoint, move the current datapoint forward until + // we're at the rightmost datapoint which is not larger than "x". + while ($idx < $max) { + if ($map_x[$idx + 1] > $x) { + break; + } + + $idx++; + $value = $map[$map_x[$idx]]; + } + + $yv[] = $value; + } + + return $yv; + } + +} diff --git a/src/applications/fact/chart/PhabricatorChartDataQuery.php b/src/applications/fact/chart/PhabricatorChartDataQuery.php index 15708341f7..7e92938e2c 100644 --- a/src/applications/fact/chart/PhabricatorChartDataQuery.php +++ b/src/applications/fact/chart/PhabricatorChartDataQuery.php @@ -34,4 +34,48 @@ final class PhabricatorChartDataQuery return $this->limit; } + public function selectInputValues(array $xv) { + $result = array(); + + $x_min = $this->getMinimumValue(); + $x_max = $this->getMaximumValue(); + $limit = $this->getLimit(); + + if ($x_min !== null) { + foreach ($xv as $key => $x) { + if ($x < $x_min) { + unset($xv[$key]); + } + } + } + + if ($x_max !== null) { + foreach ($xv as $key => $x) { + if ($x > $x_max) { + unset($xv[$key]); + } + } + } + + // If we have too many data points, throw away some of the data. + + // TODO: This doesn't work especially well right now. + + if ($limit !== null) { + $count = count($xv); + if ($count > $limit) { + $ii = 0; + $every = ceil($count / $limit); + foreach ($xv as $key => $x) { + $ii++; + if (($ii % $every) && ($ii != $count)) { + unset($xv[$key]); + } + } + } + } + + return array_values($xv); + } + } diff --git a/src/applications/fact/chart/PhabricatorChartFunction.php b/src/applications/fact/chart/PhabricatorChartFunction.php index 40d13b0c38..414147da56 100644 --- a/src/applications/fact/chart/PhabricatorChartFunction.php +++ b/src/applications/fact/chart/PhabricatorChartFunction.php @@ -3,11 +3,7 @@ abstract class PhabricatorChartFunction extends Phobject { - private $xAxis; - private $yAxis; - private $argumentParser; - private $sourceFunction; final public function getFunctionKey() { return $this->getPhobjectClassConstant('FUNCTIONKEY', 32); @@ -44,13 +40,73 @@ abstract class PhabricatorChartFunction $parser->setHaveAllArguments(true); $parser->parseArguments(); - $source_argument = $parser->getSourceFunctionArgument(); - if ($source_argument) { - $source_function = $this->getArgument($source_argument->getName()); - $this->setSourceFunction($source_function); + return $this; + } + + public function getSubfunctions() { + $result = array(); + $result[] = $this; + + foreach ($this->getFunctionArguments() as $argument) { + foreach ($argument->getSubfunctions() as $subfunction) { + $result[] = $subfunction; + } } - return $this; + return $result; + } + + public function getFunctionArguments() { + $results = array(); + + $parser = $this->getArgumentParser(); + foreach ($parser->getAllArguments() as $argument) { + if ($argument->getType() !== 'function') { + continue; + } + + $name = $argument->getName(); + $value = $this->getArgument($name); + + if (!is_array($value)) { + $results[] = $value; + } else { + foreach ($value as $arg_value) { + $results[] = $arg_value; + } + } + } + + return $results; + } + + public function newDatapoints(PhabricatorChartDataQuery $query) { + $xv = $this->newInputValues($query); + + if ($xv === null) { + $xv = $this->newDefaultInputValues($query); + } + + $xv = $query->selectInputValues($xv); + + $n = count($xv); + $yv = $this->evaluateFunction($xv); + + $points = array(); + for ($ii = 0; $ii < $n; $ii++) { + $y = $yv[$ii]; + + if ($y === null) { + continue; + } + + $points[] = array( + 'x' => $xv[$ii], + 'y' => $y, + ); + } + + return $points; } abstract protected function newArguments(); @@ -73,96 +129,26 @@ abstract class PhabricatorChartFunction return $this->argumentParser; } + abstract public function evaluateFunction(array $xv); + + public function getDomain() { + return null; + } + + public function newInputValues(PhabricatorChartDataQuery $query) { + return null; + } + public function loadData() { return; } - protected function setSourceFunction(PhabricatorChartFunction $source) { - $this->sourceFunction = $source; - return $this; - } - - protected function getSourceFunction() { - return $this->sourceFunction; - } - - final public function setXAxis(PhabricatorChartAxis $x_axis) { - $this->xAxis = $x_axis; - return $this; - } - - final public function getXAxis() { - return $this->xAxis; - } - - final public function setYAxis(PhabricatorChartAxis $y_axis) { - $this->yAxis = $y_axis; - return $this; - } - - final public function getYAxis() { - return $this->yAxis; - } - - protected function canEvaluateFunction() { - return false; - } - - protected function evaluateFunction($x) { - throw new PhutilMethodNotImplementedException(); - } - - public function hasDomain() { - if ($this->canEvaluateFunction()) { - return false; - } - - throw new PhutilMethodNotImplementedException(); - } - - public function getDatapoints(PhabricatorChartDataQuery $query) { - if ($this->canEvaluateFunction()) { - $points = $this->newSourceDatapoints($query); - foreach ($points as $key => $point) { - $y = $point['y']; - $y = $this->evaluateFunction($y); - $points[$key]['y'] = $y; - } - - return $points; - } - - return $this->newDatapoints($query); - } - - protected function newDatapoints(PhabricatorChartDataQuery $query) { - throw new PhutilMethodNotImplementedException(); - } - - protected function newSourceDatapoints(PhabricatorChartDataQuery $query) { - $source = $this->getSourceFunction(); - if ($source) { - return $source->getDatapoints($query); - } - - return $this->newDefaultDatapoints($query); - } - - protected function newDefaultDatapoints(PhabricatorChartDataQuery $query) { + protected function newDefaultInputValues(PhabricatorChartDataQuery $query) { $x_min = $query->getMinimumValue(); $x_max = $query->getMaximumValue(); $limit = $query->getLimit(); - $points = array(); - $steps = $this->newLinearSteps($x_min, $x_max, $limit); - foreach ($steps as $step) { - $points[] = array( - 'x' => $step, - 'y' => $step, - ); - } - - return $points; + return $this->newLinearSteps($x_min, $x_max, $limit); } protected function newLinearSteps($src, $dst, $count) { @@ -213,5 +199,4 @@ abstract class PhabricatorChartFunction return $steps; } - } diff --git a/src/applications/fact/chart/PhabricatorChartFunctionArgument.php b/src/applications/fact/chart/PhabricatorChartFunctionArgument.php index baa4ee14d0..bbcf8209ba 100644 --- a/src/applications/fact/chart/PhabricatorChartFunctionArgument.php +++ b/src/applications/fact/chart/PhabricatorChartFunctionArgument.php @@ -5,7 +5,7 @@ final class PhabricatorChartFunctionArgument private $name; private $type; - private $isSourceFunction; + private $repeatable; public function setName($name) { $this->name = $name; @@ -16,6 +16,15 @@ final class PhabricatorChartFunctionArgument return $this->name; } + public function setRepeatable($repeatable) { + $this->repeatable = $repeatable; + return $this; + } + + public function getRepeatable() { + return $this->repeatable; + } + public function setType($type) { $types = array( 'fact-key' => true, @@ -40,15 +49,6 @@ final class PhabricatorChartFunctionArgument return $this->type; } - public function setIsSourceFunction($is_source_function) { - $this->isSourceFunction = $is_source_function; - return $this; - } - - public function getIsSourceFunction() { - return $this->isSourceFunction; - } - public function newValue($value) { switch ($this->getType()) { case 'fact-key': diff --git a/src/applications/fact/chart/PhabricatorChartFunctionArgumentParser.php b/src/applications/fact/chart/PhabricatorChartFunctionArgumentParser.php index 281cb88f4d..04342ed4cc 100644 --- a/src/applications/fact/chart/PhabricatorChartFunctionArgumentParser.php +++ b/src/applications/fact/chart/PhabricatorChartFunctionArgumentParser.php @@ -11,6 +11,7 @@ final class PhabricatorChartFunctionArgumentParser private $argumentMap = array(); private $argumentPosition = 0; private $argumentValues = array(); + private $repeatableArgument = null; public function setFunction(PhabricatorChartFunction $function) { $this->function = $function; @@ -55,6 +56,32 @@ final class PhabricatorChartFunctionArgumentParser $name)); } + if ($this->repeatableArgument) { + if ($spec->getRepeatable()) { + throw new Exception( + pht( + 'Chart function "%s" emitted multiple repeatable argument '. + 'specifications ("%s" and "%s"). Only one argument may be '. + 'repeatable and it must be the last argument.', + $this->getFunctionArgumentSignature(), + $name, + $this->repeatableArgument->getName())); + } else { + throw new Exception( + pht( + 'Chart function "%s" emitted a repeatable argument ("%s"), then '. + 'another argument ("%s"). No arguments are permitted after a '. + 'repeatable argument.', + $this->getFunctionArgumentSignature(), + $this->repeatableArgument->getName(), + $name)); + } + } + + if ($spec->getRepeatable()) { + $this->repeatableArgument = $spec; + } + $this->argumentMap[$name] = $spec; $this->unparsedArguments[] = $spec; @@ -72,12 +99,26 @@ final class PhabricatorChartFunctionArgumentParser return $this; } + public function getAllArguments() { + return array_values($this->argumentMap); + } + public function parseArguments() { $have_count = count($this->rawArguments); $want_count = count($this->argumentMap); if ($this->haveAllArguments) { - if ($want_count !== $have_count) { + if ($this->repeatableArgument) { + if ($want_count > $have_count) { + throw new Exception( + pht( + 'Function "%s" expects %s or more argument(s), but only %s '. + 'argument(s) were provided.', + $this->getFunctionArgumentSignature(), + $want_count, + $have_count)); + } + } else if ($want_count !== $have_count) { throw new Exception( pht( 'Function "%s" expects %s argument(s), but %s argument(s) were '. @@ -105,6 +146,14 @@ final class PhabricatorChartFunctionArgumentParser $raw_argument = array_shift($this->unconsumedArguments); $this->argumentPosition++; + $is_repeatable = $argument->getRepeatable(); + + // If this argument is repeatable and we have more arguments, add it + // back to the end of the list so we can continue parsing. + if ($is_repeatable && $this->unconsumedArguments) { + $this->unparsedArguments[] = $argument; + } + try { $value = $argument->newValue($raw_argument); } catch (Exception $ex) { @@ -118,7 +167,14 @@ final class PhabricatorChartFunctionArgumentParser $ex->getMessage())); } - $this->argumentValues[$name] = $value; + if ($is_repeatable) { + if (!isset($this->argumentValues[$name])) { + $this->argumentValues[$name] = array(); + } + $this->argumentValues[$name][] = $value; + } else { + $this->argumentValues[$name] = $value; + } } } @@ -141,7 +197,7 @@ final class PhabricatorChartFunctionArgumentParser $argument_list[] = $key; } - if (!$this->haveAllArguments) { + if (!$this->haveAllArguments || $this->repeatableArgument) { $argument_list[] = '...'; } @@ -151,43 +207,4 @@ final class PhabricatorChartFunctionArgumentParser implode(', ', $argument_list)); } - public function getSourceFunctionArgument() { - $required_type = 'function'; - - $sources = array(); - foreach ($this->argumentMap as $key => $argument) { - if (!$argument->getIsSourceFunction()) { - continue; - } - - if ($argument->getType() !== $required_type) { - throw new Exception( - pht( - 'Function "%s" defines an argument "%s" which is marked as a '. - 'source function, but the type of this argument is not "%s".', - $this->getFunctionArgumentSignature(), - $argument->getName(), - $required_type)); - } - - $sources[$key] = $argument; - } - - if (!$sources) { - return null; - } - - if (count($sources) > 1) { - throw new Exception( - pht( - 'Function "%s" defines more than one argument as a source '. - 'function (arguments: %s). Functions must have zero or one '. - 'source function.', - $this->getFunctionArgumentSignature(), - implode(', ', array_keys($sources)))); - } - - return head($sources); - } - } diff --git a/src/applications/fact/chart/PhabricatorComposeChartFunction.php b/src/applications/fact/chart/PhabricatorComposeChartFunction.php new file mode 100644 index 0000000000..f6148ceae9 --- /dev/null +++ b/src/applications/fact/chart/PhabricatorComposeChartFunction.php @@ -0,0 +1,73 @@ +newArgument() + ->setName('f') + ->setType('function') + ->setRepeatable(true), + ); + } + + public function evaluateFunction(array $xv) { + $original_positions = array_keys($xv); + $remaining_positions = $original_positions; + foreach ($this->getFunctionArguments() as $function) { + $xv = $function->evaluateFunction($xv); + + // If a function evaluates to "null" at some positions, we want to return + // "null" at those positions and stop evaluating the function. + + // We also want to pass "evaluateFunction()" a natural list containing + // only values it should evaluate: keys should not be important and we + // should not pass "null". This simplifies implementation of functions. + + // To do this, first create a map from original input positions to + // function return values. + $xv = array_combine($remaining_positions, $xv); + + // If a function evaluated to "null" at any position where we evaluated + // it, the result will be "null". We remove the position from the + // vector so we stop evaluating it. + foreach ($xv as $x => $y) { + if ($y !== null) { + continue; + } + + unset($xv[$x]); + } + + // Store the remaining original input positions for the next round, then + // throw away the array keys so we're passing the next function a natural + // list with only non-"null" values. + $remaining_positions = array_keys($xv); + $xv = array_values($xv); + + // If we have no more inputs to evaluate, we can bail out early rather + // than passing empty vectors to functions for evaluation. + if (!$xv) { + break; + } + } + + + $yv = array(); + $xv = array_combine($remaining_positions, $xv); + foreach ($original_positions as $position) { + if (isset($xv[$position])) { + $y = $xv[$position]; + } else { + $y = null; + } + $yv[$position] = $y; + } + + return $yv; + } + +} diff --git a/src/applications/fact/chart/PhabricatorConstantChartFunction.php b/src/applications/fact/chart/PhabricatorConstantChartFunction.php index 6ce9f1942d..cdc6c9494a 100644 --- a/src/applications/fact/chart/PhabricatorConstantChartFunction.php +++ b/src/applications/fact/chart/PhabricatorConstantChartFunction.php @@ -13,12 +13,16 @@ final class PhabricatorConstantChartFunction ); } - protected function canEvaluateFunction() { - return true; - } + public function evaluateFunction(array $xv) { + $n = $this->getArgument('n'); - protected function evaluateFunction($x) { - return $this->getArgument('n'); + $yv = array(); + + foreach ($xv as $x) { + $yv[] = $n; + } + + return $yv; } } diff --git a/src/applications/fact/chart/PhabricatorCosChartFunction.php b/src/applications/fact/chart/PhabricatorCosChartFunction.php index 213124c3d1..04b8041fdb 100644 --- a/src/applications/fact/chart/PhabricatorCosChartFunction.php +++ b/src/applications/fact/chart/PhabricatorCosChartFunction.php @@ -6,20 +6,17 @@ final class PhabricatorCosChartFunction const FUNCTIONKEY = 'cos'; protected function newArguments() { - return array( - $this->newArgument() - ->setName('x') - ->setType('function') - ->setIsSourceFunction(true), - ); + return array(); } - protected function canEvaluateFunction() { - return true; - } + public function evaluateFunction(array $xv) { + $yv = array(); - protected function evaluateFunction($x) { - return cos(deg2rad($x)); + foreach ($xv as $x) { + $yv[] = cos(deg2rad($x)); + } + + return $yv; } } diff --git a/src/applications/fact/chart/PhabricatorFactChartFunction.php b/src/applications/fact/chart/PhabricatorFactChartFunction.php index 2f28b22335..ea59d3459e 100644 --- a/src/applications/fact/chart/PhabricatorFactChartFunction.php +++ b/src/applications/fact/chart/PhabricatorFactChartFunction.php @@ -6,7 +6,7 @@ final class PhabricatorFactChartFunction const FUNCTIONKEY = 'fact'; private $fact; - private $datapoints; + private $map; protected function newArguments() { $key_argument = $this->newArgument() @@ -44,73 +44,46 @@ final class PhabricatorFactChartFunction return; } - $points = array(); + $map = array(); + foreach ($data as $row) { + $value = (int)$row['value']; + $epoch = (int)$row['epoch']; - $sum = 0; - foreach ($data as $key => $row) { - $sum += (int)$row['value']; - $points[] = array( - 'x' => (int)$row['epoch'], - 'y' => $sum, - ); - } - - $this->datapoints = $points; - } - - public function getDatapoints(PhabricatorChartDataQuery $query) { - $points = $this->datapoints; - if (!$points) { - return array(); - } - - $x_min = $query->getMinimumValue(); - $x_max = $query->getMaximumValue(); - $limit = $query->getLimit(); - - if ($x_min !== null) { - foreach ($points as $key => $point) { - if ($point['x'] < $x_min) { - unset($points[$key]); - } + if (!isset($map[$epoch])) { + $map[$epoch] = 0; } + + $map[$epoch] += $value; } - if ($x_max !== null) { - foreach ($points as $key => $point) { - if ($point['x'] > $x_max) { - unset($points[$key]); - } - } - } - - // If we have too many data points, throw away some of the data. - if ($limit !== null) { - $count = count($points); - if ($count > $limit) { - $ii = 0; - $every = ceil($count / $limit); - foreach ($points as $key => $point) { - $ii++; - if (($ii % $every) && ($ii != $count)) { - unset($points[$key]); - } - } - } - } - - return $points; - } - - public function hasDomain() { - return true; + $this->map = $map; } public function getDomain() { - // TODO: We can examine the data to fit a better domain. + return array( + head_key($this->map), + last_key($this->map), + ); + } - $now = PhabricatorTime::getNow(); - return array($now - phutil_units('90 days in seconds'), $now); + public function newInputValues(PhabricatorChartDataQuery $query) { + return array_keys($this->map); + } + + public function evaluateFunction(array $xv) { + $map = $this->map; + + $yv = array(); + + foreach ($xv as $x) { + if (isset($map[$x])) { + $yv[] = $map[$x]; + } else { + $yv[] = null; + } + } + + return $yv; } } diff --git a/src/applications/fact/chart/PhabricatorHigherOrderChartFunction.php b/src/applications/fact/chart/PhabricatorHigherOrderChartFunction.php new file mode 100644 index 0000000000..519e602a80 --- /dev/null +++ b/src/applications/fact/chart/PhabricatorHigherOrderChartFunction.php @@ -0,0 +1,56 @@ +getFunctionArguments() as $function) { + $domain = $function->getDomain(); + if ($domain !== null) { + list($min, $max) = $domain; + $minv[] = $min; + $maxv[] = $max; + } + } + + if (!$minv && !$maxv) { + return null; + } + + $min = null; + $max = null; + + if ($minv) { + $min = min($minv); + } + + if ($maxv) { + $max = max($maxv); + } + + return array($min, $max); + } + + public function newInputValues(PhabricatorChartDataQuery $query) { + $map = array(); + foreach ($this->getFunctionArguments() as $function) { + $xv = $function->newInputValues($query); + if ($xv !== null) { + foreach ($xv as $x) { + $map[$x] = true; + } + } + } + + if (!$map) { + return null; + } + + ksort($map); + + return array_keys($map); + } + +} diff --git a/src/applications/fact/chart/PhabricatorScaleChartFunction.php b/src/applications/fact/chart/PhabricatorScaleChartFunction.php index 78540a6844..0fdcd4d64d 100644 --- a/src/applications/fact/chart/PhabricatorScaleChartFunction.php +++ b/src/applications/fact/chart/PhabricatorScaleChartFunction.php @@ -7,22 +7,22 @@ final class PhabricatorScaleChartFunction protected function newArguments() { return array( - $this->newArgument() - ->setName('x') - ->setType('function') - ->setIsSourceFunction(true), $this->newArgument() ->setName('scale') ->setType('number'), ); } - protected function canEvaluateFunction() { - return true; - } + public function evaluateFunction(array $xv) { + $scale = $this->getArgument('scale'); - protected function evaluateFunction($x) { - return $x * $this->getArgument('scale'); + $yv = array(); + + foreach ($xv as $x) { + $yv[] = $x * $scale; + } + + return $yv; } } diff --git a/src/applications/fact/chart/PhabricatorShiftChartFunction.php b/src/applications/fact/chart/PhabricatorShiftChartFunction.php index 52f33d26b5..8b53d34277 100644 --- a/src/applications/fact/chart/PhabricatorShiftChartFunction.php +++ b/src/applications/fact/chart/PhabricatorShiftChartFunction.php @@ -7,22 +7,22 @@ final class PhabricatorShiftChartFunction protected function newArguments() { return array( - $this->newArgument() - ->setName('x') - ->setType('function') - ->setIsSourceFunction(true), $this->newArgument() ->setName('shift') ->setType('number'), ); } - protected function canEvaluateFunction() { - return true; - } + public function evaluateFunction(array $xv) { + $shift = $this->getArgument('shift'); - protected function evaluateFunction($x) { - return $x * $this->getArgument('shift'); + $yv = array(); + + foreach ($xv as $x) { + $yv[] = $x + $shift; + } + + return $yv; } } diff --git a/src/applications/fact/chart/PhabricatorSinChartFunction.php b/src/applications/fact/chart/PhabricatorSinChartFunction.php index 1ac557f868..26a37bfd82 100644 --- a/src/applications/fact/chart/PhabricatorSinChartFunction.php +++ b/src/applications/fact/chart/PhabricatorSinChartFunction.php @@ -6,20 +6,17 @@ final class PhabricatorSinChartFunction const FUNCTIONKEY = 'sin'; protected function newArguments() { - return array( - $this->newArgument() - ->setName('x') - ->setType('function') - ->setIsSourceFunction(true), - ); + return array(); } - protected function canEvaluateFunction() { - return true; - } + public function evaluateFunction(array $xv) { + $yv = array(); - protected function evaluateFunction($x) { - return sin(deg2rad($x)); + foreach ($xv as $x) { + $yv[] = sin(deg2rad($x)); + } + + return $yv; } } diff --git a/src/applications/fact/chart/PhabricatorSumChartFunction.php b/src/applications/fact/chart/PhabricatorSumChartFunction.php new file mode 100644 index 0000000000..88d12eba62 --- /dev/null +++ b/src/applications/fact/chart/PhabricatorSumChartFunction.php @@ -0,0 +1,40 @@ +newArgument() + ->setName('f') + ->setType('function') + ->setRepeatable(true), + ); + } + + public function evaluateFunction(array $xv) { + $fv = array(); + foreach ($this->getFunctionArguments() as $function) { + $fv[] = $function->evaluateFunction($xv); + } + + $n = count($xv); + $yv = array_fill(0, $n, null); + + foreach ($fv as $f) { + for ($ii = 0; $ii < $n; $ii++) { + if ($f[$ii] !== null) { + if (!isset($yv[$ii])) { + $yv[$ii] = 0; + } + $yv[$ii] += $f[$ii]; + } + } + } + + return $yv; + } + +} diff --git a/src/applications/fact/chart/PhabricatorXChartFunction.php b/src/applications/fact/chart/PhabricatorXChartFunction.php deleted file mode 100644 index b2b7ab36ed..0000000000 --- a/src/applications/fact/chart/PhabricatorXChartFunction.php +++ /dev/null @@ -1,20 +0,0 @@ -setArguments(array($argv)); + } - $functions[] = id(new PhabricatorFactChartFunction()) - ->setArguments(array('tasks.count.create')); + $subfunctions = array(); + foreach ($functions as $function) { + foreach ($function->getSubfunctions() as $subfunction) { + $subfunctions[] = $subfunction; + } + } - $functions[] = id(new PhabricatorFactChartFunction()) - ->setArguments(array('tasks.open-count.create')); - - $x_function = id(new PhabricatorXChartFunction()) - ->setArguments(array()); - - $functions[] = id(new PhabricatorConstantChartFunction()) - ->setArguments(array(360)); - - $functions[] = id(new PhabricatorSinChartFunction()) - ->setArguments(array($x_function)); - - $cos_function = id(new PhabricatorCosChartFunction()) - ->setArguments(array($x_function)); - - $functions[] = id(new PhabricatorShiftChartFunction()) - ->setArguments( - array( - array( - 'scale', - array( - 'cos', - array( - 'scale', - array('x'), - 0.001, - ), - ), - 10, - ), - 200, - )); + foreach ($subfunctions as $subfunction) { + $subfunction->loadData(); + } list($domain_min, $domain_max) = $this->getDomain($functions); @@ -63,11 +70,7 @@ final class PhabricatorFactChartController extends PhabricatorFactController { $datasets = array(); foreach ($functions as $function) { - $function->setXAxis($axis); - - $function->loadData(); - - $points = $function->getDatapoints($data_query); + $points = $function->newDatapoints($data_query); $x = array(); $y = array(); @@ -157,19 +160,18 @@ final class PhabricatorFactChartController extends PhabricatorFactController { private function getDomain(array $functions) { $domain_min_list = null; $domain_max_list = null; + foreach ($functions as $function) { - if ($function->hasDomain()) { - $domain = $function->getDomain(); + $domain = $function->getDomain(); - list($domain_min, $domain_max) = $domain; + list($function_min, $function_max) = $domain; - if ($domain_min !== null) { - $domain_min_list[] = $domain_min; - } + if ($function_min !== null) { + $domain_min_list[] = $function_min; + } - if ($domain_max !== null) { - $domain_max_list[] = $domain_max; - } + if ($function_max !== null) { + $domain_max_list[] = $function_max; } }