Refactor SubjectSuggestion to store subject_id - not uri, label, notation #604
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #604 +/- ##
==========================================
+ Coverage 99.52% 99.55% +0.03%
==========================================
Files 86 86
Lines 5695 5673 -22
==========================================
- Hits 5668 5648 -20
+ Misses 27 25 -2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
SonarCloud Quality Gate failed.
|
juhoinkinen
approved these changes
Aug 10, 2022
This was referenced Aug 12, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is a pretty major refactoring and simplification (hopefully!) of how subject suggestions are represented internally in Annif. There are two main representations, VectorSuggestionResult and ListSuggestionResult. The latter consists of SubjectSuggestion objects, which is a named tuple that has until now contained the URI, label and notation of the suggested subject as well as its suggestion score.
This PR simplifies SubjectSuggestion so that apart from the score, it only contains a numeric subject ID instead of all the subject attributes (uri, label, notation). The subject ID can be looked up from the SubjectIndex when more information about the subject is needed for example in the CLI and REST API. Since subject IDs are already used in many backends as well as other functionality, this usually makes the code simpler. It is also now easier to convert between vector and list representations of suggestions, since both now use numeric subject IDs and there is no need to rely on a SubjectIndex for the conversions.
There are also related changes in the AnnifVocabulary and SubjectIndex classes, for example the SubjectIndex now makes use of the Subject namedtuple instead of plain tuples, which makes its API a bit cleaner.
I have not yet tested this extensively but instead I've relied on the unit tests, which have of course been modified accordingly. I would expect this to bring small improvements in efficiency.