-
-
Notifications
You must be signed in to change notification settings - Fork 2
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
Updated get_books #122
Updated get_books #122
Conversation
get_books now accepts ";" as a delimeter and allows for book ranges, just as get_chapters does.
@ddaspit @johnml1135 The tests are failing because I chose to raise a ValueError on an invalid id, instead of a RuntimeError, to be consistent with get_chapters and parse_selection. Should I just update the exceptions to be RuntimeErrors or should the tests be updated? I assume the former, but I feel that ValueError is a more informative and consistent error message. |
-Now tests for book ranges -Now tests for ValueErrors instead of RuntimeErrors with an incorrect input
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #122 +/- ##
==========================================
+ Coverage 88.12% 88.14% +0.01%
==========================================
Files 273 273
Lines 15989 16010 +21
==========================================
+ Hits 14091 14112 +21
Misses 1898 1898 ☔ View full report in Codecov by Sentry. |
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.
Reviewed 1 of 1 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @TaperChipmunk32)
machine/scripture/parse.py
line 45 at r2 (raw file):
def update_selection(book_set: Set[int], book_nums: Set[int], subtraction: bool) -> Set[int]:
This should be prefixed with an underscore, since it is a "private" function.
machine/scripture/parse.py
line 50 at r2 (raw file):
book_set.difference_update(book_nums) else: book_ids = set()
This can be simplified using list comprehension.
-Renamed update_selection to _update_selection since it is "private" -Simplified _update_selection
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.
Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @TaperChipmunk32)
get_books now accepts ";" as a delimeter and allows for book ranges, just as get_chapters does.
This change is