-
Notifications
You must be signed in to change notification settings - Fork 359
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: assign less than min threshold students to previous center #51
base: main
Are you sure you want to change the base?
refactor: assign less than min threshold students to previous center #51
Conversation
Abstracts the center allocation to a separate function Assigns the remaining students to previous center if the remainder is less than minimum threshold
0fa5027
to
d42d315
Compare
if next_allot < MIN_STUDENT_IN_CENTER and len(allocated_centers) > 0: | ||
# get the first center code | ||
allocate_center_code = centers[0]['cscode'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
first center may not always be the best choice to allocate remaining students.
- it may not have required capacity as we have used center_cap_left for current_center.
- in my opinion assigning them to last allotted center might be preferable for registration number distribution. eg: school with 408 students - Case 1: Center A: registration no 1 - 200; Center B reg no 201 - 408. Case 2: Center A: registration no. 1 - 208; Center B reg no 209 - 408. Case 1 would be preferred as starting registration number for all centers is a easy number
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sapradhan Thank you for your feedback. I agree with your point that the distributed number might not be cleaner.
I have one question, so when assigning the registration number, we will not care about the offset as below:
- Center A: reg number from 1 to 200
- Center B: reg number from 201 to 400
- Remaining reg number ( 401 to 408 )
Let me know and I will make changes to assign to the last center.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handling issue 1 should be good enough for now.
I realize now that issue 2 may not be that important.
Registration numbers distribution are not currently handled by this script. I only brought up registration numbers looking at notice published by NEB, so may be part of a future enhancement. Also centers can be ordered differently so as to make the registration no. distribution cleaner. eg - if we allot Center A - 208, Center B - 200, it does not mean we have to put first 208 students to Center A. We can distribute Center B 1 - 200, Center A 201 - 408. So I take back my previous opinion that assigning students to last allotted center is preferred.
The new implementation fixes the #13 of allocating small amount of students to a new center.
Details
calc_per_center
MIN_STUDENT_IN_CENTER
), they are assigned to the first center of that specific school.MIN_STUDENT_IN_CENTER
), they are assigned to a new centerThe PR also abstract the center allocation logic to a new function
allocate_to_centers
.I saw that there is a WIP PR related to the unit test. I can also add the test for
allocate_to_centers
function once the that PR is merged to make sure that the allocation is working as expected.Resolves #13