From 27ae9af4badc50cca7b4570564d6c259957af1f0 Mon Sep 17 00:00:00 2001 From: David Monllao Date: Tue, 13 Jun 2017 13:39:00 +0200 Subject: [PATCH] MDL-57791 analytics: No UI in calculables APIs --- analytics/classes/calculable.php | 30 ++++++++++++++ analytics/classes/local/indicator/binary.php | 20 ++++++++-- .../classes/local/indicator/discrete.php | 17 +++++--- analytics/classes/local/indicator/linear.php | 20 ++++++++-- analytics/classes/local/target/base.php | 8 ---- analytics/classes/local/target/binary.php | 10 ++--- analytics/classes/local/target/discrete.php | 40 ++++++++++++++----- analytics/classes/local/target/linear.php | 6 +-- report/insights/classes/output/insight.php | 39 +++++++++++++++++- 9 files changed, 152 insertions(+), 38 deletions(-) diff --git a/analytics/classes/calculable.php b/analytics/classes/calculable.php index 34f26fc4af2..5559fcfe97a 100644 --- a/analytics/classes/calculable.php +++ b/analytics/classes/calculable.php @@ -35,6 +35,12 @@ defined('MOODLE_INTERNAL') || die(); */ abstract class calculable { + const OUTCOME_NEUTRAL = 0; + const OUTCOME_VERY_POSITIVE = 1; + const OUTCOME_OK = 2; + const OUTCOME_NEGATIVE = 3; + const OUTCOME_VERY_NEGATIVE = 4; + /** * @var array[] */ @@ -81,6 +87,30 @@ abstract class calculable { $this->sampledata = array(); } + /** + * Returns the visible value of the calculated value. + * + * @param float $value + * @param string|false $subtype + * @return string + */ + public function get_display_value($value, $subtype = false) { + return $value; + } + + /** + * Returns how good the calculated value is. + * + * Use one of \core_analytics\calculable::OUTCOME_* values. + * + * @param float $value + * @param string|false $subtype + * @return int + */ + public function get_calculation_outcome($value, $subtype = false) { + throw new \coding_exception('Please overwrite get_calculation_outcome method'); + } + /** * Retrieve the specified element associated to $sampleid. * diff --git a/analytics/classes/local/indicator/binary.php b/analytics/classes/local/indicator/binary.php index 410adf4fff6..a44e669f547 100644 --- a/analytics/classes/local/indicator/binary.php +++ b/analytics/classes/local/indicator/binary.php @@ -41,6 +41,13 @@ abstract class binary extends discrete { return array(0); } + /** + * get_display_value + * + * @param float $value + * @param string $subtype + * @return string + */ public function get_display_value($value, $subtype = false) { // No subtypes for binary values by default. @@ -53,13 +60,20 @@ abstract class binary extends discrete { } } - public function get_value_style($value, $subtype = false) { + /** + * get_calculation_outcome + * + * @param float $value + * @param string $subtype + * @return int + */ + public function get_calculation_outcome($value, $subtype = false) { // No subtypes for binary values by default. if ($value == -1) { - return 'alert alert-warning'; + return self::OUTCOME_NEGATIVE; } else if ($value == 1) { - return 'alert alert-info'; + return self::OUTCOME_OK; } else { throw new \moodle_exception('errorpredictionformat', 'analytics'); } diff --git a/analytics/classes/local/indicator/discrete.php b/analytics/classes/local/indicator/discrete.php index b1b6168ab8b..7f63e9104f9 100644 --- a/analytics/classes/local/indicator/discrete.php +++ b/analytics/classes/local/indicator/discrete.php @@ -67,11 +67,11 @@ abstract class discrete extends base { /** * Returns the value to display when the prediction is $value. * - * @param mixed $value + * @param float $value * @param string $subtype - * @return void + * @return string */ - public function get_display_value($value, $subtype) { + public function get_display_value($value, $subtype = false) { $displayvalue = array_search($subtype, static::get_classes()); @@ -81,10 +81,17 @@ abstract class discrete extends base { return $displayvalue; } - public function get_display_style($value, $subtype) { + /** + * get_display_style + * + * @param float $ignoredvalue + * @param string $ignoredsubtype + * @return int + */ + public function get_display_style($ignoredvalue, $ignoredsubtype) { // No style attached to indicators classes, they are what they are, a cat, // a horse or a sandwich, they are not good or bad. - return ''; + return \core_analytics\calculable::OUTCOME_NEUTRAL; } protected function to_features($calculatedvalues) { diff --git a/analytics/classes/local/indicator/linear.php b/analytics/classes/local/indicator/linear.php index 222df2b3916..bdc9f6f6909 100644 --- a/analytics/classes/local/indicator/linear.php +++ b/analytics/classes/local/indicator/linear.php @@ -64,16 +64,30 @@ abstract class linear extends base { return true; } + /** + * get_display_value + * + * @param float $value + * @param string $subtype + * @return string + */ public function get_display_value($value, $subtype = false) { $diff = static::get_max_value() - static::get_min_value(); return round(100 * ($value - static::get_min_value()) / $diff) . '%'; } - public function get_value_style($value, $subtype = false) { + /** + * get_calculation_outcome + * + * @param float $value + * @param string $subtype + * @return int + */ + public function get_calculation_outcome($value, $subtype = false) { if ($value < 0) { - return 'alert alert-warning'; + return self::OUTCOME_NEGATIVE; } else { - return 'alert alert-info'; + return self::OUTCOME_OK; } } diff --git a/analytics/classes/local/target/base.php b/analytics/classes/local/target/base.php index e31fad869c5..0f6d4b28122 100644 --- a/analytics/classes/local/target/base.php +++ b/analytics/classes/local/target/base.php @@ -120,14 +120,6 @@ abstract class base extends \core_analytics\calculable { ); } - public function get_display_value($value) { - return $value; - } - - public function get_value_style($value) { - throw new \coding_exception('Please overwrite \core_analytics\local\target\base::get_value_style'); - } - /** * Callback to execute once a prediction has been returned from the predictions processor. * diff --git a/analytics/classes/local/target/binary.php b/analytics/classes/local/target/binary.php index b77ca9ca1d9..055e28d0148 100644 --- a/analytics/classes/local/target/binary.php +++ b/analytics/classes/local/target/binary.php @@ -62,7 +62,7 @@ abstract class binary extends discrete { return array(0); } - public function get_value_style($value) { + public function get_calculation_outcome($value, $ignoredsubtype = false) { if (!self::is_a_class($value)) { throw new \moodle_exception('errorpredictionformat', 'analytics'); @@ -71,14 +71,14 @@ abstract class binary extends discrete { if (in_array($value, $this->ignored_predicted_classes())) { // Just in case, if it is ignored the prediction should not even be recorded but if it would, it is ignored now, // which should mean that is it nothing serious. - return 'alert alert-success'; + return self::OUTCOME_VERY_POSITIVE; } - // Default binaries are danger when prediction = 1. + // By default binaries are danger when prediction = 1. if ($value) { - return 'alert alert-danger'; + return self::OUTCOME_VERY_NEGATIVE; } - return 'alert alert-success'; + return self::OUTCOME_VERY_POSITIVE; } protected static function classes_description() { diff --git a/analytics/classes/local/target/discrete.php b/analytics/classes/local/target/discrete.php index b6033b85db6..e05b673d835 100644 --- a/analytics/classes/local/target/discrete.php +++ b/analytics/classes/local/target/discrete.php @@ -44,7 +44,14 @@ abstract class discrete extends base { return (in_array($class, static::get_classes())); } - public function get_display_value($value) { + /** + * get_display_value + * + * @param float $value + * @param string $subtype + * @return string + */ + public function get_display_value($value, $ignoredsubtype = false) { if (!self::is_a_class($value)) { throw new \moodle_exception('errorpredictionformat', 'analytics'); @@ -66,7 +73,14 @@ abstract class discrete extends base { return $descriptions[$key]; } - public function get_value_style($value) { + /** + * get_calculation_outcome + * + * @param float $value + * @param string $ignoredsubtype + * @return int + */ + public function get_calculation_outcome($value, $ignoredsubtype = false) { if (!self::is_a_class($value)) { throw new \moodle_exception('errorpredictionformat', 'analytics'); @@ -74,16 +88,16 @@ abstract class discrete extends base { if (in_array($value, $this->ignored_predicted_classes())) { // Just in case, if it is ignored the prediction should not even be recorded. - return ''; + return self::OUTCOME_OK; } - debugging('Please overwrite \core_analytics\local\target\discrete::get_value_style, all your target classes are styled ' . + debugging('Please overwrite \core_analytics\local\target\discrete::get_calculation_outcome, all your target classes are styled ' . 'the same way otherwise', DEBUG_DEVELOPER); - return 'alert alert-danger'; + return self::OUTCOME_OK; } /** - * Returns the target discrete values. + * Returns all the possible values the target calculation can return. * * Only useful for targets using discrete values, must be overwriten if it is the case. * @@ -91,12 +105,20 @@ abstract class discrete extends base { */ public static function get_classes() { // Coding exception as this will only be called if this target have non-linear values. - throw new \coding_exception('Overwrite get_classes() and return an array with the different target classes'); + throw new \coding_exception('Overwrite get_classes() and return an array with the different values the ' . + 'target calculation can return'); } + /** + * Returns descriptions for each of the values the target calculation can return. + * + * The array indexes should match self::get_classes indexes. + * + * @return array + */ protected static function classes_description() { - throw new \coding_exception('Overwrite classes_description() and return an array with the target classes description and ' . - 'indexes matching self::get_classes'); + throw new \coding_exception('Overwrite classes_description() and return an array with a description for each of the ' . + 'different values the target calculation can return. Indexes should match self::get_classes indexes'); } /** diff --git a/analytics/classes/local/target/linear.php b/analytics/classes/local/target/linear.php index 1d1ce28c498..76b6ea75f15 100644 --- a/analytics/classes/local/target/linear.php +++ b/analytics/classes/local/target/linear.php @@ -40,14 +40,14 @@ abstract class linear extends base { throw new \coding_exception('Sorry, this version\'s prediction processors only support targets with binary values.'); } - public function get_value_style($value) { + public function get_calculated_outcome($value, $ignoredsubtype = false) { // This is very generic, targets will probably be interested in overwriting this. $diff = static::get_max_value() - static::get_min_value(); if (($value - static::get_min_value()) / $diff >= 0.5) { - return 'alert alert-success'; + return self::OUTCOME_VERY_POSITIVE; } - return 'alert alert-danger'; + return self::OUTCOME_VERY_NEGATIVE; } /** diff --git a/report/insights/classes/output/insight.php b/report/insights/classes/output/insight.php index eb0ea15a722..84d5d2b1212 100644 --- a/report/insights/classes/output/insight.php +++ b/report/insights/classes/output/insight.php @@ -72,7 +72,7 @@ class insight implements \renderable, \templatable { $predictedvalue = $this->prediction->get_prediction_data()->prediction; $predictionid = $this->prediction->get_prediction_data()->id; $data->predictiondisplayvalue = $this->model->get_target()->get_display_value($predictedvalue); - $data->predictionstyle = $this->model->get_target()->get_value_style($predictedvalue); + $data->predictionstyle = $this->get_calculation_style($this->model->get_target(), $predictedvalue); $actions = $this->model->get_target()->prediction_actions($this->prediction); if ($actions) { @@ -108,11 +108,46 @@ class insight implements \renderable, \templatable { $obj = new \stdClass(); $obj->name = forward_static_call(array($calculation->indicator, 'get_name'), $calculation->subtype); $obj->displayvalue = $calculation->indicator->get_display_value($calculation->value, $calculation->subtype); - $obj->style = $calculation->indicator->get_value_style($calculation->value, $calculation->subtype); + $obj->style = $this->get_calculation_style($calculation->indicator, $calculation->value, $calculation->subtype); $data->calculations[] = $obj; } return $data; } + + /** + * Returns a CSS class from the calculated value outcome. + * + * @param \core_analytics\calculable $calculable + * @param mixed $value + * @param string|false $subtype + * @return string + */ + protected function get_calculation_style(\core_analytics\calculable $calculable, $value, $subtype = false) { + $outcome = $calculable->get_calculation_outcome($value, $subtype); + switch ($outcome) { + case \core_analytics\calculable::OUTCOME_NEUTRAL: + $style = ''; + break; + case \core_analytics\calculable::OUTCOME_VERY_POSITIVE: + $style = 'alert alert-success'; + break; + case \core_analytics\calculable::OUTCOME_OK: + $style = 'alert alert-info'; + break; + case \core_analytics\calculable::OUTCOME_NEGATIVE: + $style = 'alert alert-warning'; + break; + case \core_analytics\calculable::OUTCOME_VERY_NEGATIVE: + $style = 'alert alert-danger'; + break; + default: + throw new \coding_exception('The outcome returned by ' . get_class($calculable) . '::get_calculation_outcome is ' . + 'not one of the accepted values. Please use \core_analytics\calculable::OUTCOME_VERY_POSITIVE, ' . + '\core_analytics\calculable::OUTCOME_OK, \core_analytics\calculable::OUTCOME_NEGATIVE or ' . + '\core_analytics\calculable::OUTCOME_VERY_NEGATIVE'); + } + return $style; + } }