Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Heatmap is off-by-one #23

Open
4 tasks done
Rapptz opened this issue Aug 24, 2018 · 4 comments
Open
4 tasks done

Heatmap is off-by-one #23

Rapptz opened this issue Aug 24, 2018 · 4 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@Rapptz
Copy link
Contributor

Rapptz commented Aug 24, 2018

Prerequisite checklist

  • In case of a bug: Have you tried restarting Anki?
  • Are you running the latest version of the add-on. Have you redownloaded the add-on from AnkiWeb to make sure?
  • Did you check the add-on documentation (e.g. AnkiWeb description, Wiki if it exists) for known issues?
  • Did you perform a cursory search through the existing issue reports?

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

  • Operating system: Windows
  • Version: 8.1

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
Copy link
Owner

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 3fc50bf vs commit cfe03c2. 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.

@glutanimate glutanimate added the more info needed More information needed to address issue label Aug 24, 2018
@Rapptz
Copy link
Contributor Author

Rapptz commented Aug 25, 2018

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:

3fc50bf - Works
cfe03c2 - 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 glutanimate added this to the v0.7.0 milestone Sep 17, 2018
@glutanimate glutanimate self-assigned this Sep 17, 2018
@glutanimate glutanimate added bug Something isn't working and removed more info needed More information needed to address issue labels Sep 17, 2018
glutanimate added a commit that referenced this issue Sep 23, 2018
Fixes negative counts for future dates, as reported in #23

Made possible through wa0x6e/cal-heatmap#250
glutanimate added a commit that referenced this issue Oct 17, 2018
When calculating delta in days, make sure to use 'today' as defined by
Anki (taking "day starts at" setting into account). Dates passed on to
onClick handler are always set to midnight, so adjust calTodayDate
accordingly, and then perform calculation.

Should address some instances of the off-by-one errors reported in #23
glutanimate added a commit that referenced this issue Oct 17, 2018
glutanimate added a commit that referenced this issue Oct 17, 2018
Fixes another class of off-by-one errors that would occur in timezones
with DST switches. (related to #23)
@glutanimate
Copy link
Owner

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
Copy link
Contributor Author

Rapptz commented Oct 31, 2018

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

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants