Quickfix/Duplicate Rrobin Method

[Lancasterwu]

Description of Pull Request

Thanks for @enpuyou found this out while writing the test case for view.py.

PR#259 put the old version of group_rrobin.py back, and it was merged to the master branch which it should not. group_rrobin.py had bad design and didn't work as what it suppose to, so @paladp refactored it in PR#246 and @ALEXANDERB82 fixed the temporal dependency related to group_scoring.py in PR#222 so it is combined with group_random.py into a new file group_creation.py. With the bug, since there is no test case for group_rrobin.py, test coverage will decrease. However, the function was used by view.py so coverage report didn't catch it.

This PR fixed the duplicated method and changed views.py to make sure it is calling the right function.

Type of Change

Please describe the pull request as one of the following:

• [x] Bug fix
• [ ] Breaking change
• [ ] New feature
• [ ] Documentation update
• [ ] Other

Tags

@Lancasterwu @barrezuetai

[ibarrezueta]

We can merge this branch into the bugfix branch to get all of the changes, can't we?

[ibarrezueta]

@Lancasterwu This doesn't work with the current set up. I just made a profile and tried grouping students and I got an error from the new rrobon function.

empty range for randrange() (1,1, 0)

This line line (66)

# choose a random column from the student responses as the priority
    # column to distribute students by
    indices = list(range(0, numgrps))
    random.shuffle(indices)
    target_group = itertools.cycle(indices)

    ---> priorityColumn = random.randint(1, len(responses[0]) - 1) …

    logging.info("column priority: %d", priorityColumn)
    # iterate through the responses and check if the priority column is true
    # if it is, add that response to the next group
    for response in responses:
        if response[priorityColumn] is True:

[ibarrezueta]

I merged it with the bug fix branch, and fixed that error

[Lancasterwu]

@barrezuetai I don't it is proper to close this PR when everything is unclear.

[Lancasterwu]

We can merge this branch into the bugfix branch to get all of the changes, can't we?

@barrezuetai You can, but it is unnecessary. The outcome of master branch will be the same after both PR is merged, and the specific PR should handle the specific problem. I would stay against merging the branch into your PR since there is already clear description of the issue in this PR.

[ibarrezueta]

I've already merged this branch into the bug fix branch and changed some of the stuff there.

[Lancasterwu]

@barrezuetai I saw that. However, can you change it back, or just merge that PR after this is merged.

[Michionlion]

@Lancasterwu is right, you should refrain from merging PR branches into other PRs, as it creates nightmares for the reviewers. I will merge this to master, and then @barrezuetai you will need to merge master into your branch.