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.

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

Add calendar heatmap display format #1759

Merged
merged 21 commits into from
Oct 2, 2024
Merged

Conversation

alichtman
Copy link
Contributor

@alichtman alichtman commented Jun 21, 2023

I think this format is a lot more useful than the termgraph solution discussed in #743.

Calendar Heatmap Exporter:

image

termgraph --calendar:

image

Checklist

  • I have read the contributing doc.
  • I have included a link to the relevant issue number.
  • I have checked to ensure there aren't other open pull requests
    for the same issue.
  • I have written new tests for these changes, as needed.

@alichtman
Copy link
Contributor Author

I'm not sure how to resolve this last lint error:

$ poe lint
Poe => poetry --version
Poetry (version 1.3.2)
Poe => poetry check
All set!
Poe => flakeheaven --version
FlakeHeaven 3.3.0
Flake8    4.0.1
For plugins versions use flakeheaven plugins
Poe => flakeheaven plugins
NAME                 | VERSION  | CODES            | RULES
flake8-black         | 0.3.6    | BLK              | -BLK901
flake8-isort         | 6.0.0    | I00              | +*
flake8-simplify      | 0.20.0   | SIM              | +*
flake8-type-checking | 2.4.0    | TC               | +*
mccabe               | 0.6.1    | C90              |
pycodestyle          | 2.8.0    | E, W             | -E101, -E111, -E114, -E115, -E116, -E117, -E12*, -E13*, -E2*, -E3*, -E401, -E5*, -E70, -W1*, -W2*, -W3*, -W5*
pyflakes             | 2.4.0    | F                | +*
pylint               | 0.0.0    | C, E, F, I, R, W | +*
Poe => flakeheaven lint
jrnl/datatypes/__init__.py:1:1: F401 '.NestedDict.NestedDict' imported but unused [pyflakes]
Error: Sequence aborted after failed subtask 'flakeheaven lint'

It looks structurally identical to https://github.com/alichtman/jrnl/blob/bd921f2e0518f0051f7887d3ed42123e750fe26a/jrnl/journals/__init__.py#L1-L5

@micahellison
Copy link
Member

Thanks for the PR! I'm playing around with it and it looks nice. I like that it doesn't introduce any new dependencies, also.

I'm not sure how to resolve this last lint error

It's in pyproject.toml:161. It should pass if you change that line to this (which I think is a good change):

[tool.flakeheaven.exceptions."*/__init__.py"]

The overall construction of this PR looks nice. I think the only things left to add are some BDD tests like those in formats.feature, and some unit tests in the new utility functions.

@alichtman
Copy link
Contributor Author

alichtman commented Jul 3, 2023

I took a look at setting up some tests for this. The scaffolding for the test went into that last commit, but the test output is all misformatted, and I'm not sure why.

At first glance, it seems like the output is being wrapped because it hits a character boundary. Except not all output is wrapped. See this example specifically. Why are the July calendar dates wrapped, but not the bar at the top of the calendar? And why aren't the numbers that should be below July 2013 (No there, if the "wrap point" was at entries).

           June 2013 (2 entries)                   July 2013 (No
  entries)

        Mon Tue Wed Thu Fri Sat Sun
  Mon Tue Wed Thu Fri Sat Sun
        ━━━━━━━━━━━━━━━━━━━━━━━━━━━              ━━━━━━━━━━━━━━━━━━━━━━━━━━━
                              1
    2                1   2   3
  4   5   6   7
          3   4   5   6   7
    8   9                8   9
  10  11  12  13  14
         10  11  12  13  14
   15  16               15  16
  17  18  19  20  21
         17  18  19  20  21
   22  23               22  23
  24  25  26  27  28
         24  25  26  27  28
   29  30               29  30
  31

...

$ pytest tests/bdd/test_features.py -vv
...
================================================================= FAILURES =================================================================
________________________________________________ test_export_calendar_heatmap[simple.yaml] _________________________________________________
[gw8] linux -- Python 3.11.2 /home/alichtman/.cache/pypoetry/virtualenvs/jrnl-aPxBJCHI-py3.11/bin/python
Traceback (most recent call last):
  File "/home/alichtman/.cache/pypoetry/virtualenvs/jrnl-aPxBJCHI-py3.11/lib/python3.11/site-packages/_pytest/fixtures.py", line 909, in call_fixture_func
    fixture_result = fixturefunc(**kwargs)
                     ^^^^^^^^^^^^^^^^^^^^^
  File "/home/alichtman/Desktop/Development/projects/jrnl/tests/lib/then_steps.py", line 86, in output_should_be
    assert actual == expected
AssertionError: assert == failed. [pytest-clarity diff shown]

  LHS vs RHS shown below

  2013-06-09, 1
  2013-06-10, 1
  ───────────────────────────────────── 2013
  ─────────────────────────────────────

           June 2013 (2 entries)                   July 2013 (No
  entries)

        Mon Tue Wed Thu Fri Sat Sun
  Mon Tue Wed Thu Fri Sat Sun
        ━━━━━━━━━━━━━━━━━━━━━━━━━━━              ━━━━━━━━━━━━━━━━━━━━━━━━━━━
                              1
    2                1   2   3
  4   5   6   7
          3   4   5   6   7
    8   9                8   9
  10  11  12  13  14
         10  11  12  13  14
   15  16               15  16
  17  18  19  20  21
         17  18  19  20  21
   22  23               22  23
  24  25  26  27  28
         24  25  26  27  28
   29  30               29  30
  31


         August 2013 (No entries)                September 2013 (No
  entries)

        Mon Tue Wed Thu Fri Sat Sun
  Mon Tue Wed Thu Fri Sat Sun
        ━━━━━━━━━━━━━━━━━━━━━━━━━━━              ━━━━━━━━━━━━━━━━━━━━━━━━━━━
                      1   2   3
    4
         1
          5   6   7   8   9
   10  11                2   3
  4   5   6   7   8
         12  13  14  15  16
   17  18                9  10
  11  12  13  14  15
         19  20  21  22  23
   24  25               16  17
  18  19  20  21  22
         26  27  28  29  30
   31                   23  24  25
   26  27  28  29
                                                  30



         October 2013 (No entries)               November 2013 (No
  entries)

        Mon Tue Wed Thu Fri Sat Sun
  Mon Tue Wed Thu Fri Sat Sun
        ━━━━━━━━━━━━━━━━━━━━━━━━━━━              ━━━━━━━━━━━━━━━━━━━━━━━━━━━
              1   2   3   4
  5   6
    1   2   3
          7   8   9  10  11
   12  13                4   5
  6   7   8   9  10
         14  15  16  17  18
   19  20               11  12
  13  14  15  16  17
         21  22  23  24  25
   26  27               18  19
  20  21  22  23  24
         28  29  30  31
                     25  26  27
  28  29  30


        December 2013 (No entries)

        Mon Tue Wed Thu Fri Sat Sun
        ━━━━━━━━━━━━━━━━━━━━━━━━━━━

  1
          2   3   4   5   6
    7   8
          9  10  11  12  13
   14  15
         16  17  18  19  20
   21  22
         23  24  25  26  27
   28  29
         30  31


========================================================= short test summary info ==========================================================
FAILED tests/bdd/test_features.py::test_export_calendar_heatmap[simple.yaml] - assert == failed. [pytest-clarity diff shown]
================================================ 1 failed, 485 passed, 11 skipped in 2.55s =================================================

The following works on my machine without any weird formatting. I don't know why the test is rendering it differently:

$ jrnl --export calendar

@micahellison
Copy link
Member

That's very strange. I'm not sure what's going on. It looks like pytest is treating the output fine in the GitHub Actions tests. On my Windows 10 machine, pytest is wrapping the output at 80 characters (well, 82 actually, but there it adds a 2-space indent before the output).

Maybe there's something in your environment that it's reading through pytest? You might try it in the isolated tox environment:

poetry shell
tox -- tests/bdd/test_features.py::test_export_calendar_heatmap -vv

When I do that, it wraps to the full width of my terminal (then breaks because pytest adds 2 more spaces to indent the output, but at least that's explicable).

As for the root cause, I wonder if it has something to do with Rich's method for getting the terminal size. If you're not running Windows, it investigates STDIN, STDERR, and STDOUT for terminal size (console.py:1016), and uses the first terminal size it can get without an error. Maybe there's something going on there?

@alichtman
Copy link
Contributor Author

I'm not sure when I'll have time to look into this.

@alichtman
Copy link
Contributor Author

Following up on our Signal conversation: Since this is a non-critical component of the codebase, and none of us are going to invest the time in getting these tests working, I'm proposing shipping this diff without the tests.

@alichtman alichtman marked this pull request as ready for review January 15, 2024 23:32
@alichtman
Copy link
Contributor Author

All tests pass.

$ poetry run poe test
Poe => poetry --version
Poetry (version 1.3.2)
Poe => poetry check
All set!
Poe => ruff --version
ruff 0.1.3
Poe => ruff .
Poe => black --version
black, 23.10.1 (compiled: yes)
Python (CPython) 3.11.6
Poe => black --check .
All done! ✨ 🍰 ✨
74 files would be left unchanged.
Poe => tox -q -e py --
================================================================= test session starts ==================================================================
platform linux -- Python 3.11.6, pytest-7.4.4, pluggy-1.3.0
cachedir: .tox/py/.pytest_cache
rootdir: /home/alichtman/Desktop/Development/projects/jrnl
configfile: pyproject.toml
plugins: xdist-3.5.0, bdd-7.0.1
16 workers [679 items]
ss.........................................s.......sss.......................................................................................... [ 21%]
..................................s...................................................................................s.sss...................... [ 42%]
................................................................................................................................................ [ 63%]
................................................................................................................................................ [ 84%]
................s....s.......ss...............s.....ssss.sssssssssssssssss..s..s...........s..........                                           [100%]
=========================================================== 639 passed, 40 skipped in 2.55s ============================================================
  py: OK (4.59 seconds)
  congratulations :) (4.62 seconds)

@wren
Copy link
Member

wren commented Feb 2, 2024

@alichtman Hey, we're just getting everything caught up after being away for a while (updated dependencies today). We'll get this PR reviewed soon. Thanks for your patience!

@alichtman
Copy link
Contributor Author

Addressed lints and re-tested.
image

Copy link
Member

@micahellison micahellison left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for your patience!

@micahellison micahellison added the enhancement New feature or request label Oct 2, 2024
@micahellison micahellison linked an issue Oct 2, 2024 that may be closed by this pull request
@micahellison micahellison changed the title Add calendar heatmap exporter Add calendar heatmap display format Oct 2, 2024
@micahellison micahellison merged commit a8bd0bc into jrnl-org:develop Oct 2, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add daily streak report with calendar heatmap
3 participants