search

glutanimate / review-heatmap

Anki add-on to help you keep track of your review activity
https://ankiweb.net/shared/info/1771074083
Other
1.2k stars 89 forks source link

Heatmap is off-by-one #23

Open Rapptz opened 6 years ago

Rapptz commented 6 years ago

Prerequisite checklist

What is the problem/feature you would like to see fixed/implemented?

Heatmap seems to have an off-by-one error in its visual calculations. This happens on any type of heatmap view (specific deck, entire collection).

How can we reproduce the problem?

  1. Hover over an entry in a heatmap
  2. Read the tooltip
  3. Click on it.

Expected behavior: To actually show the reviews for that day

Actual behavior: It shows the reviews for the day prior (e.g. seen:2 instead of seen:1 and prop:due=1 instead of prop:due=2, etc).

Reproducible?: Yep.

Examples:

If you hover over today and double-click it:

You'll get a search with seen:2 instead of the correct seen:1.

The same thing happens with future dates e.g.

This actually says there are -30 cards due (which makes no sense) and opens a search for prop:due=2 (which is correct) but there are no 30 cards for that day. Instead, prop:due=3 (the next day) does have those 30 cards.

Version information

Anki

Version 2.1.2 Qt 5.9.6 PyQt 5.9.2

System

Other

I'm not sure if this is a bug you're already aware of since I am technically running on untested unreleased code, but I figured it'd be worth opening an issue anyway.

I'm also not sure if it matters, but I have my Anki setting to consider a new day 16 hours after midnight.

glutanimate commented 6 years ago

Hi,

Thanks for the report!

The off-by-one issue is likely to be a side-effect of your new day cut-off time. Do you still have an Anki 2.0 installation handy? If so, it would be very helpful if you could compare the master branch at commit 3fc50bf42fd4933bc64d48a4d15c90badde0dd89 vs commit cfe03c290a3d5a0e7c0492f688e786a18f29fb12. The latter introduced some changes in the day cutoff handling and it would be interesting to see how these affect this issue.

Another interesting observation would be to check how the add-on behaves before and after your daily cut-off time. Do the seen / prop:due values change at the cutoff?

As for the negative counts, that's just because the master branch is currently in a transitional state where it's not really meant to be used outside of development. However, I still appreciate your taking the time to report this. In the future I might switch to using feature branches or a dev branch instead, so that the master branch remains somewhat functional. FWIW, if you'd just like to have a working version of the add-on on 2.1 you might want to check out this commit instead.

Rapptz commented 6 years ago

Hello.

I installed Anki 2.0 again to help test this. Both commits seem to work fine actually. Likewise, the master commit works fine now on Anki 2.1. So I believe your secondary observation might have a better effect on this. When I opened the issue it was before my cut-off time, but now with everything fixed it's after my cut-off time.

Edit:

Now that it's past midnight local time, the drift has happened again. So I decided to take this opportunity to re-test the extension.

On Anki 2.0:

https://github.com/glutanimate/review-heatmap/commit/3fc50bf42fd4933bc64d48a4d15c90badde0dd89 - Works https://github.com/glutanimate/review-heatmap/commit/cfe03c290a3d5a0e7c0492f688e786a18f29fb12 - Broken

Using feature branches is a good idea in general, since you want a branch to be at a state where it's usable as a checkpoint without referring to a specific commit. Tags work fine for this purpose too but branches are better for features.

I'm fine with using whatever commit, I just wanted to see if I could help test. I do not actually mind the working state of the add-on overall (afterall, I can just do a checkout to get a more stable version).

glutanimate commented 5 years ago

Hey @Rapptz,

Thanks again for the detailed bug report.

I've just pushed out the first beta release of v0.7.0 which should contain a number of fixes for this and other time-related issues. Would you mind giving it a test run to see if it resolves the off-by-one situation you were seeing? (only if you have time of course!)

Rapptz commented 5 years ago

I can't get the build to work at all.

Anki 2.1.2 Python 3.6.1 Qt 5.9.6 PyQt 5.9.2
Platform: Windows 8.1
Flags: frz=True ao=True sv=1

Caught exception:
  File "aqt\deckbrowser.py", line 91, in __renderPage
  File "<decorator-gen-4>", line 2, in _renderStats
  File "anki\hooks.py", line 66, in decorator_wrapper
  File "anki\hooks.py", line 63, in repl
  File "AppData\Roaming\Anki2\addons21\review_heatmap\views.py", line 92, in deckbrowserRenderStats
    html = ret + hmap.generate(view="deckbrowser")
  File "AppData\Roaming\Anki2\addons21\review_heatmap\heatmap.py", line 100, in generate
    if prefs["display"][view]:
<class 'TypeError'>: list indices must be integers or slices, not str