Skip to content
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

Improve professor merging #84

Merged
merged 3 commits into from
Feb 6, 2023
Merged

Improve professor merging #84

merged 3 commits into from
Feb 6, 2023

Conversation

nsandler1
Copy link
Member

Improvements include:

  • Taking advantage of django caching by storing the many querysets in variables
  • When querying for existing aliases, don't filter on the professor. Only filter on the alias itself. Aliases are unique and therefore can't be duplicated at all
  • Not creating aliases for professors who have the exact same name as other professors in the DB. For example, there are two professors named Douglas Hamilton, but they are different people. One teaches Astronomy and one teaches Physics. Since the names are identical, any aliases would likely be identical as well. This poses a problem because if an alias were given to, say, the Astronomy professor, then anytime you use that alias you're always gonna get Douglas Hamilton for Astronomy which might not be the professor you want.
    Don't fret though, I've ensured in other parts of the codebase that aliases are never used in this case so there's no harm if one of these professors did get an alias, but I'd rather not clutter the table.

@nsandler1 nsandler1 added the priority Should be considered a priority label Feb 6, 2023
@tybug tybug merged commit 69689d7 into master Feb 6, 2023
@tybug tybug deleted the improve-prof-merge branch February 6, 2023 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority Should be considered a priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants