limited schedules #301
limited schedules #301
Conversation
|
|
👇 Click on the image for a new way to code review
Legend |
| @@ -59,7 +59,7 @@ def test_example(): | |||
| ] | |||
| # TODO: Uncomment this assertion and delete the empty list assert | |||
| # assert actual == expected | |||
| assert actual == [] | |||
| assert actual == expected | |||
|
|
|||
|
|
|||
| # TODO: Add more test cases for your logic | |||
There was a problem hiding this comment.
What are some more test cases that we should add? And how comfortable do you feel writing new test cases based on the example given to you in this file?
Brainstorm in words what test cases you think we should add, then I can comment your plan to let you know if you are missing any important ones or if any cases are not necessary. Then you can go ahead and take that plan to write the tests in Python.
| # Maximum number of available slots in a user's schedule for them to be | ||
| # eligible for a limited availability match. | ||
| self.max_available = max_available | ||
| super().__init__(name=GENERATOR_LIMITED_SCHEDULES) | ||
|
|
||
| def generate(self, inp: MatchingInput) -> Iterator[Match]: | ||
| # TODO: Implement your generator | ||
| inp.logger.info("Luke Arrington was here!") | ||
|
|
There was a problem hiding this comment.
Write now the outline of your code looks like:
- Get the users who have limited schedules
- Compare limited users to other users, check if their schedules align, and create matches
- Return matches (work in progress)
There is a lot going on in step two, so I recommend splitting it up like this:
- Get the users who have limited schedules
- Compare limited users to other users, collect the schedule times they have in common (save to a dictionary)
- Loop through the dictionary to create matches for the users you saved
- Return matches
This should also help reduce the deep levels of indentation in your code. Sometimes too much indentation can make it harder to read and follow the code. That might be a clue that the logic is getting too complicated.
There was a problem hiding this comment.
Great work! We also improved the indentation issues by using a continue in the loop.
| for l_availability in limited_user.schedule: | ||
| for availability in user.schedule: | ||
| if ( | ||
| l_availability == availability | ||
| ): # if the availabilities in limited user schedule and other user schedule are the same, then we have a match! |
There was a problem hiding this comment.
Right now, I think what you are doing is getting just one time the two users have in common. We want to get all the times they have in common. It should be a small list since one user has limited availability. But, since the parameter max_available can change, we shouldn't assume that it will always be 1.
I would recommend creating a dictionary like common_availability where the key is the set of the two user IDs and the value is a list of the availabilities they have in common. When you go through this loop, you can append the matching availabilities to that list.
| matches[ | ||
| {limited_user.uid, user.uid} | ||
| ] = Match( | ||
| users={limited_user.uid, user.uid} |
There was a problem hiding this comment.
When you are ready to create the matches, you can add the availabilities the two users have in common like this:
# Put this at the top of your file to import the metadata (or add it to the list of types already being imported)
from pipeline.types import MatchMetadata
Match(
users={uid1, uid2},
metadata=MatchMetadata(
generator=GENERATOR_LIMITED_SCHEDULES,
availability=availability
)
)The variable availability should be a list of the matching availabilities between the users, from their schedules.
vingkan
added
enhancement
| # compare limited users to all users | ||
| for limited_user in limited_availability: | ||
| for user in inp.users: | ||
| userid_tuple = (limited_user.uid, user.uid) |
There was a problem hiding this comment.
There's also a subtle issue here that if both users are limited users, we might create two versions of the same match, one where the first user comes first in the tuple, and one with the opposite order.
This is okay because the matching engine ignores duplicate matches. But we can fix this in the future when you write more test cases.
* Luke Arrington's first commit. * compares users and add matches to dict * added create_matches and test * completed create function * completed create matches function * passed test cases
* Luke Arrington's first commit. * compares users and add matches to dict * added create_matches and test * completed create function * completed create matches function * passed test cases
No description provided.