London | 25-SDC-Nov | Emiliano Uruena | Sprint 5 | Laptops Allocation#307
London | 25-SDC-Nov | Emiliano Uruena | Sprint 5 | Laptops Allocation#307Emilianouz wants to merge 3 commits intoCodeYourFuture:mainfrom
Conversation
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). If this PR needs reviewed, please add the 'Needs Review' label to this PR after you have resolved the issues listed above. |
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). If this PR needs reviewed, please add the 'Needs Review' label to this PR after you have resolved the issues listed above. |
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). If this PR needs reviewed, please add the 'Needs Review' label to this PR after you have resolved the issues listed above. |
6f464c6 to
964c731
Compare
|
README file no necessary. It was removed. |
|
Made review, interesting approach but there are some errors in the implementation itself worthy fixing + the algorithm won't find the most minimal possible configuration |
Implement calcutate_sandess and backtracking for laptop allocation based on user preferences and minimize sadness.
|
I'll leave @SlideGauge to carry on with this since he's most recent reviewer |
SlideGauge
left a comment
There was a problem hiding this comment.
Overall really good solution, please change the returning type of the function and I will mark the review as complete.
laptop-allocation.py
Outdated
| for laptop in laptops: | ||
| if (laptop.operating_system == os and not laptop.assigned): | ||
| allocation[person.name] = laptop.id | ||
| assigned = True |
There was a problem hiding this comment.
Does this assigned variable prevent us from selecting the same laptop several times?
laptop-allocation.py
Outdated
| allocation[person.name] = laptop.id | ||
| assigned = True | ||
| break | ||
| if assigned: |
There was a problem hiding this comment.
what happens if we did not found the laptop after the first loop
for laptop in laptops?
laptop-allocation.py
Outdated
| allocation[person.name] = laptop.id | ||
| assigned = True | ||
| break | ||
| if assigned: |
There was a problem hiding this comment.
if assigned was set to True, what happens after iterating the first os?
laptop-allocation.py
Outdated
| operating_system: OperatingSystem | ||
| assigned: bool = False | ||
|
|
||
| def allocate_laptops(people: List[Person], laptops: List[Laptop]) -> Dict[str, int]: |
There was a problem hiding this comment.
The algorithm belongs to the type of algorithms called "Greedy", i.e. on each step the algorithm takes the most suitable choice seen at this moment, not globally.
Consider:
Imran wants UBUNTU, ARCH
Eliza wants UBUNTU
The algorithm will give UBUNTU to Imran and nothing to Eliza, leading to sadness 100.
But we can invent different algorithm: if we distribute UBUNTU to Eliza and ARCH to Imran, we will have sadness 1. This algorithm will be slower, but our task is not to create the quickest algorithm, but minimizing the sadness.
Hint: this task requires analyzing of all possible permutations of laptops distribution.
| name: str | ||
| age: int | ||
| preferred_operating_system: List[OperatingSystem] | ||
| assigned_laptop: Optional[int] = None |
There was a problem hiding this comment.
Since the dataclass is frozen, we won't be able to change the value of this variable
laptop-allocation.py
Outdated
| return person.preferred_operating_system.index(laptop.operating_system) | ||
| return 100 | ||
|
|
||
| def allocate_laptops(people: List[Person], laptops: List[Laptop]) -> Dict[str, int]: |
There was a problem hiding this comment.
Really nice, yes, backtrack will solve the task.
laptop-allocation.py
Outdated
| return person.preferred_operating_system.index(laptop.operating_system) | ||
| return 100 | ||
|
|
||
| def allocate_laptops(people: List[Person], laptops: List[Laptop]) -> Dict[str, int]: |
There was a problem hiding this comment.
One thing to take into account:
the task demands us for the function to have this signature:
def allocate_laptops(people: List[Person], laptops: List[Laptop]) -> Dict[Person, Laptop]:
Note, we return Dict of Person->Laptop, not str->int. It is valuable to take into account, as if we just return str->int, we won't be able to distinguish to people with the same name.
There was a problem hiding this comment.
Right. We should return Dict[Person, Laptop] so people with the same name are handled correctly.
…on: Person and Laptop objects Change allocation function to change the returning type of the function: Person and Laptop objects
|
@SlideGauge , I changed it to return type of the function: Person and Laptop objects. Thank you |
|
Very nice, thank you, accepted |
Self checklist
Changelist
Implementing Laptop allocation exercise.