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 totals report #18

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
Draft

Add totals report #18

wants to merge 12 commits into from

Conversation

meshy
Copy link
Collaborator

@meshy meshy commented Nov 29, 2022

This adds:

  • New isort and flake8 configs.
  • Very basic arg parsing. (More on this below.)
  • A help command.
  • A totals command. This produces a basic summary of a previously created JSON report file.

This lacks (todo):

Question: Is it time to break this into modules?
Question: Would it be better to use Typer (or similar) instead?

Copy link
Member

@Tenzer Tenzer left a comment

Choose a reason for hiding this comment

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

Broadly speaking, this looks good.

I'd suggest using argparse for the argument parsing. It's part of the standard library and would handle any edge cases for us.

I think we should rename the errors subcommand to parse as it's more descriptive of what it does.
At the same time we could require the parse subcommand was provided and bump the version to 0.2.0, to avoid having to maintain the legacy way without any subcommand provided - especially considering how few people presumably are using this for now.

@meshy
Copy link
Collaborator Author

meshy commented Jan 30, 2023

Thanks :)

argparse is a good idea. I've now written a commit that uses it, and it's definitely nicer.

I'm not able to push it right now because I no longer have permissions on this repo. Are you happy granting me permissions on this repo again? If not that's fine. Let me know, and I'll fork and create a new PR.

No need to rush that decision. There's still plenty more for me to do on this before I'm happy with it.

@Tenzer
Copy link
Member

Tenzer commented Jan 31, 2023

I have invited you as a collaborator on the repo.

@meshy
Copy link
Collaborator Author

meshy commented Jan 31, 2023

Thanks!

At the same time, I've renamed the `errors` subcommand to `parse`, and
dropped the `help` command in favour of the `--help` flag that
`argparse` automatically generates.
@Tenzer
Copy link
Member

Tenzer commented Feb 18, 2023

I guess this PR can be closed now?

@meshy
Copy link
Collaborator Author

meshy commented Feb 18, 2023

I'm going to re-work this. I'd still like a command that can:

  • Print the number of errors in the report, and the number of files with errors.
  • Print the history of that same data (as json or csv) over time, so that graphs can easily be produced.

@Tenzer
Copy link
Member

Tenzer commented Mar 5, 2023

I had some free time and thought I would see if I could help this along, and came up with this:

diff --git a/mypy_json_report.py b/mypy_json_report.py
index 05992c3..149ecd4 100644
--- a/mypy_json_report.py
+++ b/mypy_json_report.py
@@ -13,11 +13,13 @@
 # limitations under the License.
 
 import argparse
+import csv
 import enum
 import itertools
 import json
 import operator
 import pathlib
+import subprocess
 import sys
 import textwrap
 from collections import Counter, defaultdict
@@ -90,6 +92,26 @@ def main() -> None:
 
     parse_parser.set_defaults(func=_parse_command)
 
+    report_parser = subparsers.add_parser(
+        "report", help="Generate a report from the ratchet file."
+    )
+    report_parser.add_argument("ratchet_file", help="Path to the ratchet file.")
+    report_parser.add_argument(
+        "-g",
+        "--git",
+        action="store_true",
+        help="Generate historic report by using the history from Git.",
+    )
+    report_parser.add_argument(
+        "-o",
+        "--output-file",
+        type=argparse.FileType("w"),
+        default=sys.stdout,
+        help="The file to write the report to. If omitted, the report will be written to STDOUT. Only used in historic mode.",
+    )
+
+    report_parser.set_defaults(func=_report_command)
+
     parsed = parser.parse_args()
     parsed.func(parsed)
 
@@ -295,5 +317,65 @@ class ChangeTracker:
         return None
 
 
+def _count_errors(errors: dict[str, dict[str, int]]) -> dict[str, int]:
+    total_errors = sum(sum(file_errors.values()) for file_errors in errors.values())
+    files_with_errors = len(errors.keys())
+
+    return {
+        "total_errors": total_errors,
+        "files_with_errors": files_with_errors,
+    }
+
+
+def _report_command(args: argparse.Namespace) -> None:
+    if not args.git:
+        with open(args.ratchet_file) as file_pointer:
+            errors = json.load(file_pointer)
+
+        print(
+            "Total errors: {total_errors}\nFiles with errors: {files_with_errors}".format(
+                **_count_errors(errors)
+            )
+        )
+        return None
+
+    result = subprocess.run(
+        [
+            "git",
+            "log",
+            r"--format=%H|%ad",
+            "--date=format:%F %T",
+            "--reverse",
+            "--",
+            args.ratchet_file,
+        ],
+        check=True,
+        capture_output=True,
+    )
+    writer = csv.DictWriter(
+        args.output_file,
+        fieldnames=["commit_hash", "timestamp", "total_errors", "files_with_errors"],
+    )
+    writer.writeheader()
+
+    for line in filter(None, result.stdout.decode().split("\n")):
+        commit_hash, timestamp = line.split("|")
+        result = subprocess.run(
+            ["git", "show", f"{commit_hash}:./{args.ratchet_file}"],
+            check=True,
+            capture_output=True,
+        )
+        errors = json.loads(result.stdout.decode())
+
+        try:
+            errors_count = _count_errors(errors)
+        except AttributeError:
+            continue
+
+        writer.writerow(
+            {"commit_hash": commit_hash, "timestamp": timestamp} | errors_count
+        )
+
+
 if __name__ == "__main__":
     main()

It's pretty rough and is bound to need changes to work with Python 3.7, but it might serve as some inspiration.

I opted for shelling out to call Git as that would mean we don't need any extra dependencies, but thinking about it, it probably would be nicer to have a Git Python library to handle the Git interactions and specify that as an optional dependency.

@meshy
Copy link
Collaborator Author

meshy commented Mar 7, 2023

Thanks for that! I think shelling out to git is a reasonable approach for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants