This has been generated running the following Sniffs, all
them part of the Moodle's CodeSniffer standard:
- PSR12.Functions.ReturnTypeDeclaration
- PSR12.Functions.NullableTypeDeclaration
- moodle.Methods.MethodDeclarationSpacing
- Squiz.Whitespace.ScopeKeywordSpacing
All them are, exclusively, about correct spacing, so the changes
are, all them, only white space changes.
Only exceptions to the above are 3 changes what were setting the
return type in a new line, and, when that happens, the closing
parenthesis (bracket) has to go to the same line than the colon.
\core_analytics\model::create and \core_analytics\model::enable methods
are called from install/upgrade scripts. Therefore, they should not
include any capability check. Their capability checks are actually
redundant, as the user capabilities are already checked in the
entry-point scripts.
The original implementation did not work well for checking that the
given model does NOT exist. If no record was found in the
analytics_models table, the code execution continued and it reached the
moment when indicators were checked. If no indicators were provided, the
call ended up with error 'array_keys() expects parameter 1 to be array,
boolean given' (because indicators were set to false).
When the API had originally been designed, it was assumed that the
presence of the time splitting method implies no requirement for the
machine learning backend and that it is a sign of less performance
demanding models. So it seemed to be safe and useful to have such models
automatically enabled. That assumption did not prove to be valid. Most
of the time is spent calculating indicators that depend on log tables.
We realized it would be useful to be able to specify the default time
splitting method without the need to have such models auto-enabled.
Static predictions models (i.e. those using a target based on
assumptions, not facts) are always considered as trained. Clearing them
must not mark them as untrained. Doing so would make them being skipped
by the prediction scheduled task.
This commit reviews all continue uses in core happening within a
loop / switch / case hierarchy. This does not cover:
- Changes to libraries. Will be handled in another issue / commit.
- Uses out from loops, will be reviewed by other commit.
The policy followed has been:
- When possible, take rid of the continue.
- When clearly the intention was to jump to next element in loop
change to continue 2
- When it was not clear, keep old behavior switching to break, no
matter how weird the behavior may be.
- Basic unit test for minimum machine learning backends requirements
- Warning return messages now include not enough data
- Clear models when the predictions processor is changed
- Refined the name of a couple of constants / methods
Eloy Lafuente (stronk7)