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.

Sign up for GitHub

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

Jump to bottom

Add deck and note options. #12

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Add deck and note options. #12

Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
28766a3
Add deck options via new OptionsGroup class.
holocronweaver Aug 17, 2017
eab7246
Add per-note options and a deck creation time.
holocronweaver Aug 18, 2017
685d962
Ensure deck creation time is an integer.
holocronweaver Aug 18, 2017
ebacc3c
Fix a few typos and a minor interface inconsistency.
holocronweaver Aug 18, 2017
cd261da
Rename note.level -> stage to reduce ambiguity.
holocronweaver Aug 18, 2017
6eb44fa
Minor coding style improvement.
holocronweaver Aug 19, 2017
96606ec
Document deck and note options.
holocronweaver Aug 24, 2017
0fa1870
Internally document note and deck options further.
holocronweaver Aug 24, 2017
a5a8d00
Fix a couple field name typos in apgk_col.
holocronweaver Aug 24, 2017
4d0449a
Update tests to create OptionGroups.
holocronweaver Aug 24, 2017
61b18b5
Add Card.status to allow suspending and burying cards.
holocronweaver Aug 28, 2017
bda7097
Move note options to CardOptions and try to match database field names.
holocronweaver Sep 1, 2017
8286fbc
Merge 'upstream/master' into options.
holocronweaver Sep 1, 2017
0a13191
Remove CardOptions and set options directly on Card or via Note.
holocronweaver Sep 2, 2017
d9fd190
Avoid relative commits in __init__.py
holocronweaver Sep 17, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Add Card.status to allow suspending and burying cards.
  • Loading branch information
@holocronweaver
holocronweaver committed Aug 28, 2017
commit 61b18b5ad043c2b4c0894ef147fad825b7ae4334
11 changes: 8 additions & 3 deletions genanki/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,15 +159,16 @@ def __init__(self, ord_):
self.interval = 0

def write_to_db(self, cursor, now_ts, deck_id, note_id,
stage, due, interval, ease, reps_til_grad):
stage, queue, due, interval, ease, reps_til_grad):
cursor.execute('INSERT INTO cards VALUES(null,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?);', (
note_id, # nid - note ID
deck_id, # did - deck ID
self.ord, # ord - which card template it corresponds to
now_ts, # mod - modification time as seconds since Unix epoch
-1, # usn - value of -1 indicates need to push to server
stage, # type - 0=new, 1=learning, 2=review
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be called type. In general, we should always use the same variable naming that Anki does. If Anki's naming is confusing (which it often is), detailed comments will help make the field's purpose clear to the user.

Ditto for all the other properties here.

Copy link
Contributor Author

@holocronweaver holocronweaver Aug 29, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean just within this function, or in the user-facing API as well?
If the latter, I think of this as 'spreading the disease', where one unintuitive interface gives rise to another, but I can understand wanting a close correspondence for debugging. I doubt many users will be familiar with the Anki DB column names, so I don't believe it will make the lib easier to learn. So...I'll make the change, but I'll feel dirty, lol.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I remember now why I didn't use type for a variable name: it's a Python reserved keyword. Perhaps we should make an exception in this case?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to closely match Anki's naming. I want genanki to be a lower-level library that more-or-less directly maps to the way Anki represents data. Plus, the user will probably have to end up learning the Anki names anyway, for example if they're digging into the collections.anki2 SQLite file to debug something. Abstractions are leaky more often than people expect.

I's OK to use type as the name. If you need to access the real type() function, you can import the builtins module and call builtins.type. (Previously I also had an instance of this, calling a param ord_ instead of ord. But I changed it because it's silly to convolute your external API for the convenience of internal code, and I wasn't using ord() anyway).

stage, # queue - same as type unless buried
queue, # queue - same as type, but
# -1=suspended, -2=user buried, -3=sched buried
due, # due - new: unused
# learning: due time as integer seconds since Unix epoch
# review: integer days relative to deck creation
Expand Down Expand Up @@ -200,6 +201,9 @@ def __init__(self, model=None, fields=None, sort_field=None, tags=None, guid=Non
"""SRS learning stage.
0 = new, 1 = learning, 2 = review."""
self.stage = 0
"""SRS queue status modifiers.
0 = normal, 1 = suspended, 2 = user buried, 3 = scheduler buried"""
self.status = 0
"""Behavior depends on learning stage of note.
new: unused.
learning: due time as integer seconds since Unix epoch.
Expand Down Expand Up @@ -259,9 +263,10 @@ def write_to_db(self, cursor, now_ts, deck_id):
))

note_id = cursor.lastrowid
queue = -self.status if self.status else self.stage
for card in self.cards:
card.write_to_db(cursor, now_ts, deck_id, note_id,
self.stage, self.due, self.interval,
self.stage, queue, self.due, self.interval,
self.ease, self.reps_til_grad)

def _format_fields(self):
Expand Down