Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4.7, 2.5.3, 2.6
    • Fix Version/s: 2.5.5, 2.6.2
    • Component/s: Questions, Quiz
    • Labels:
    • Testing Instructions:
      Hide

      I added a test for the subq calculations to 2.6 and 2.7 but to backport these tests to older versions would take some time. The older versions' stats calculations are covered by unit tests though, just not the actual subq calculations. The older tests at least prove that nothing is broken by the fix and the newer ones prove that the stats for subquestions are now correct.

      Show
      I added a test for the subq calculations to 2.6 and 2.7 but to backport these tests to older versions would take some time. The older versions' stats calculations are covered by unit tests though, just not the actual subq calculations. The older tests at least prove that nothing is broken by the fix and the newer ones prove that the stats for subquestions are now correct.
    • Affected Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull Master Branch:
      MDL-43369-master

      Description

      There is an error in the code that calculates stats for subquestions (a randomly selected question). All the stats calculations should be calculated over just the attempts where the sub question appears but the code uses the average of total grades over all attempts at the question in covariance and variance calculations.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            timhunt Tim Hunt added a comment -

            Looks good. Thanks Jamie. Submitting for integration.

            Show
            timhunt Tim Hunt added a comment - Looks good. Thanks Jamie. Submitting for integration.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Hi guys,

            this is just a message to share with you that I'm going to perform a test of the automated pre-checker against all the current issues awaiting integration (16 issues).

            So, soon, you'll get some extra comments in this issue with some information from the pre-checker. Note it's not final, but just an experiment and there are lots of things to improve, from the message itself to various false positives in the checkers. So take any report with caution, it's not 100% accurate yet.

            Please, feel free to comment any idea/objection @ MDLSITE-2662. I'll be collecting everything there.

            TIA and ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Hi guys, this is just a message to share with you that I'm going to perform a test of the automated pre-checker against all the current issues awaiting integration (16 issues). So, soon, you'll get some extra comments in this issue with some information from the pre-checker. Note it's not final, but just an experiment and there are lots of things to improve, from the message itself to various false positives in the checkers. So take any report with caution, it's not 100% accurate yet. Please, feel free to comment any idea/objection @ MDLSITE-2662 . I'll be collecting everything there. TIA and ciao
            Hide
            cibot CiBoT added a comment -

            Results for MDL-43369

            Show
            cibot CiBoT added a comment - Results for MDL-43369 Branch MDL-43369 -24 to be integrated into upstream MOODLE_24_STABLE Executed job http://ci.stronk7.com/job/Precheck%20remote%20branch/710 Execution status: 0 Details: http://ci.stronk7.com/job/Precheck%20remote%20branch/710/artifact/work/smurf.html Branch MDL-43369 -25 to be integrated into upstream MOODLE_25_STABLE Executed job http://ci.stronk7.com/job/Precheck%20remote%20branch/711 Execution status: 0 Details: http://ci.stronk7.com/job/Precheck%20remote%20branch/711/artifact/work/smurf.html Branch MDL-43369 -26 to be integrated into upstream MOODLE_26_STABLE Executed job http://ci.stronk7.com/job/Precheck%20remote%20branch/712 Execution status: 0 Details: http://ci.stronk7.com/job/Precheck%20remote%20branch/712/artifact/work/smurf.html Branch MDL-43369 -master to be integrated into upstream master Executed job http://ci.stronk7.com/job/Precheck%20remote%20branch/713 Execution status: 0 Details: http://ci.stronk7.com/job/Precheck%20remote%20branch/713/artifact/work/smurf.html
            Hide
            damyon Damyon Wiese added a comment -

            Thanks Jamie,

            There are a couple of phpdocs errors that need updating in this patch.

            get_latest_steps still lists $summarksavg as part of the return even though that has been removed.

            calculated::$summarksaverage variable is not documented.

            Also -

            get_latest_steps in the class testable_question_statistics looks like it should be changed to match it's parent class (only return 2 elements in the array).

            Can you make these changes and resubmit?

            Thanks!

            Show
            damyon Damyon Wiese added a comment - Thanks Jamie, There are a couple of phpdocs errors that need updating in this patch. get_latest_steps still lists $summarksavg as part of the return even though that has been removed. calculated::$summarksaverage variable is not documented. Also - get_latest_steps in the class testable_question_statistics looks like it should be changed to match it's parent class (only return 2 elements in the array). Can you make these changes and resubmit? Thanks!
            Hide
            cibot CiBoT added a comment -

            Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

            Show
            cibot CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
            Hide
            jamiesensei Jamie Pratt added a comment -

            Resubmitting for integration after fixing issues pointed out in last integration review. Thanks for catching those Damyon!

            Show
            jamiesensei Jamie Pratt added a comment - Resubmitting for integration after fixing issues pointed out in last integration review. Thanks for catching those Damyon!
            Hide
            cibot CiBoT added a comment -

            Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!

            Show
            cibot CiBoT added a comment - Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks Jamie - this has been integrated now.

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks Jamie - this has been integrated now.
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks for fixing this Jamie,

            All unit test pass without any problem.

            Passing...

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks for fixing this Jamie, All unit test pass without any problem. Passing...
            Hide
            samhemelryk Sam Hemelryk added a comment -

            This weeks weekly release is now available and includes your code.
            A big pat on the back to you again for once more being a cog in the Moodle machine.

            Best wishes, the Moodle integration team.

            Show
            samhemelryk Sam Hemelryk added a comment - This weeks weekly release is now available and includes your code. A big pat on the back to you again for once more being a cog in the Moodle machine. Best wishes, the Moodle integration team.

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  10/Mar/14