Add lecture participation tracker using GitHub Actions (#2459) #2463

bjornthiberg · 2024-09-11T18:54:27Z

Description

This PR introduces a script and a GitHub Actions workflow to track lecture participation.

The structure of the script and workflow YAML file is based on the existing workflow for updating the Statistics of Student Registrations. It has a similar structure, can be run locally in the same way, and uses the same dependencies.

How it's supposed to work:

GitHub actions workflow

The workflow is triggered on new issue comments, and proceeds if the issue title is “Lecture participation tracker 2024”.
track_participation.py is run

Python script (track_participation.py):

Fetches all comments to the issue from the GitHub API
If the author is a collaborator, or the comment is outside the commenting window (5 hours from the start of each lecture), the comment is ignored.
Valid comments are stored in a dictionary, with keys for each student who has commented.
The script generates a markdown table, similar to the one for tracking tasks, which displays the lectures each student has attended.
The issue description is updated with this markdown table and a timestamp.

I have tested that (in a copy of the repo):

the workflow triggers correctly when a new comment is added to the tracker.
comments made by collaborators are ignored as expected.
comments made outside of the allowed window are not counted.
valid comments update the participation tracker issue description.
timezones work as expected (don't quote me on this)

I have not tested for more than two unique students/commenters, as would be the case in "production".

Updating for newer years should only require changing the lecture dates and start hours in the JSON config and the issue number in the YAML file.

Closes #2459

algomaster99

Hi! Thank you so much for the wonderful PR ❤️ Please see the changes that I have suggested.

What is the default option in your script? --printPlain, --printMarkdown, --publish? Running it without output any options simply exits the script after 3 seconds with code 0.
Do you agree to support maintenance of this script until the end of the course? :) I ask this because there could be a high chance that I merge this PR and then action breaks so I would need your help to fix it :D

@monperrus @Deee92 @javierron @sofiabobadilla example:

+-------+---------------------+-----------------------------+---------------------+
| Index |     Student Name    | Number of Lectures Attended | Lecture(s) attended |
+-------+---------------------+-----------------------------+---------------------+
|   1   |       RafDevX       |              2              |        L2 L3        |
|   2   |       Glassar       |              1              |          L2         |
|   3   |    diogotcorreia    |              2              |        L2 L3        |
|   4   |   MateusMarinheiro  |              2              |        L2 L3        |
|   5   |     bjornthiberg    |              1              |          L2         |
|   6   |       t-sorger      |              2              |        L2 L3        |
|   7   |      jackma-00      |              2              |        L2 L3        |
|   8   |    randomicecube    |              2              |        L2 L3        |
|   9   |        dvavd        |              2              |        L2 L3        |
|   10  |       Tian0602      |              2              |        L2 L3        |
|   11  |      Atheer2104     |              2              |        L2 L3        |
|   12  |      arejula27      |              2              |        L2 L3        |
|   13  |       d-melita      |              2              |        L2 L3        |
|   14  |        M1l0d        |              2              |        L2 L3        |
|   15  | CoordinatesNotFound |              2              |        L2 L3        |
|   16  |    marcocampione    |              2              |        L2 L3        |
|   17  |     Stellariser     |              2              |        L2 L3        |
|   18  |        NoelMT       |              2              |        L2 L3        |
|   19  |     pablo-freire    |              1              |          L2         |
|   20  |  GustavHenningsson  |              1              |          L2         |
|   21  |     Pesteves2002    |              2              |        L2 L3        |
|   22  |     muhammad1928    |              2              |        L2 L3        |
|   23  |       Uqqasha       |              2              |        L2 L3        |
|   24  |       Flopalot      |              1              |          L2         |
|   25  |      bepp-boop      |              1              |          L3         |
|   26  |   Alexanderliu2002  |              1              |          L3         |
|   27  |     laullaurado     |              1              |          L3         |
|   28  |       Jakebobs      |              1              |          L3         |
|   29  |     einbergisak     |              1              |          L3         |
|   30  |      Samkth123      |              1              |          L3         |
|   31  |        kthfre       |              1              |          L3         |
|   32  |      peremateu      |              1              |          L3         |
|   33  |        HexuL        |              1              |          L3         |
+-------+---------------------+-----------------------------+---------------------+

.github/workflows/lecture_participation.yml

tools/track_participation.py

bjornthiberg · 2024-09-12T14:12:39Z

Hi @algomaster99, thanks for the feedback!

I just did a force push to get the PR in line with the task structure, by changing the description and adding a task readme file (did not change the python script or YAML file).

I also added a short documentation to the /tools/README.md file, didn't see that earlier.

No default option, it just does the fetching but prints an empty string. Doesn't make a lot of sense, I'll make it print plain text as default and remove it as an option instead.
Of course!

I'll get on your other comments asap.

bjornthiberg · 2024-09-12T15:31:02Z

@algomaster99

I have addressed all your comments now I think! Let me know if I missed something or anything else needs to be adjusted :)

I have tested the code again, and the CI job on my test repo.

algomaster99

did a force push to get the PR in line with the task structure, by changing the description and adding a task readme file

Always create a PR from a different branch. You can avoid rebasing in this way.

Of course!

Thank you!

I think we are almost there! Just a few more changes.

contributions/open-source/bthiberg/README.md

algomaster99 · 2024-09-12T18:52:23Z

tools/README.md

@@ -58,3 +58,32 @@ This script is used to grade each student according to the number of task comple

 ### Usage
 ...
+
+
+## track_participation.py


I almost forgot about suggesting this change, but I am happy you noticed! Could you please also add a heading about the prerequisites that should be updated every year?

addressed in 71c9c9f

algomaster99 · 2024-09-12T18:56:51Z

tools/track_participation.py

+REPO_FULLNAME = os.getenv("REPO_FULLNAME")
+ISSUE_NUMBER = os.getenv("TRACKER_ISSUE_NUMBER")
+
+config = json.load(open("tools/track_participation_config.json"))


Suggested change

config = json.load(open("tools/track_participation_config.json"))

config = json.load(open("track_participation_config.json"))

I think this should be sufficient?

It will work when running locally from the tools folder, but the workflow will not find the file as it runs Python in the repo's root dir.

See this failed run in the test repo.

Could change that in the workflow file, but I thought this was cleaner.

Could change that in the workflow file, but I thought this was cleaner.

I think otherwise because the this path is only true with respect to the CI job so it should be in workflow file only.

algomaster99 · 2024-09-12T18:57:25Z

tools/track_participation.py

+# -*- coding: utf-8 -*-
+# Filename: track_participation.py


May I ask what these headers are for? Is it required for some plugin that you use? I am curious.

Just something I saw in the other python scripts in the repo. I believe the first line is for encoding, but is generally a remnant from python 2 as 3+ uses utf-8 by default. Second line is just for readability I suppose.

algomaster99 · 2024-09-12T19:02:26Z

tools/track_participation.py

+    lecture_start_hour = LECTURE_DATES_TO_START_TIME[lecture_date_str]
+
+    allowed_period_start = comment_time.replace(hour=lecture_start_hour, minute=0, second=0, microsecond=0)
+    allowed_period_end = allowed_period_start + timedelta(hours=COMMENTING_DURATION_HOURS)


Sorry if I being too nitpicky here, but I would request you to have two variables here instead of one COMMENT_DURATION_HOURS. One which is the actual duration and the other which is the leeway. This would be my personal opinion in terms of code readability/understandability.

Agreed! addressed in ef44d53

contributions/open-source/bthiberg/README.md

bjornthiberg · 2024-09-12T21:10:02Z

Always create a PR from a different branch. You can avoid rebasing in this way.

Damnit, didn't think about this since it's a fork. Could change now but probably more trouble than it is worth since rebasing will not be very complicated.

I think we are almost there! Just a few more changes.

On it!

bjornthiberg · 2024-09-12T21:44:26Z

@algomaster99 I've addressed the requested changes. Let me know if there’s anything else needed or if you spot any issues! :)

I suppose my commits should be squashed and rebased onto the current 2024 branch. Lmk if I should do that and force push to this PR, to make merging easier.

algomaster99

Final change and I will merge it once you resolve it :)

algomaster99 · 2024-09-12T21:53:16Z

tools/track_participation.py

+REPO_FULLNAME = os.getenv("REPO_FULLNAME")
+ISSUE_NUMBER = os.getenv("TRACKER_ISSUE_NUMBER")
+
+config = json.load(open("tools/track_participation_config.json"))


Could change that in the workflow file, but I thought this was cleaner.

I think otherwise because the this path is only true with respect to the CI job so it should be in workflow file only.

algomaster99 · 2024-09-12T22:11:18Z

Thanks for the quick fix! Let's give it a run, shall we? I am looking forward to it.

bjornthiberg · 2024-09-12T22:14:09Z

I squeezed in another quick fix! The timestamp ended up being incorrect with the timezone change, should be fixed now in 5c90115

algomaster99 · 2024-09-12T22:15:14Z

The timestamp ended up being incorrect with the timezone change, should be fixed now in 5c90115

Awesome! I did not notice it. Thanks!

algomaster99 · 2024-09-12T22:18:33Z

Are we ready to merge? 😄

algomaster99 · 2024-09-12T22:28:09Z

Merging! Let's hope nothing breaks :D

algomaster99 · 2024-09-12T22:35:38Z

Great work! One cool thing I immediately realised was that the table could be sorted by the number of lectures attended. Could you also propose a PR for that? I hope it is not too much effort.

bjornthiberg · 2024-09-14T14:38:22Z

Should be easy! Will do next week

algomaster99 · 2024-09-14T14:40:15Z

Thanks!

algomaster99 requested changes Sep 12, 2024

View reviewed changes

feat: add lecture participation tracker using GitHub Actions

d8ac9a1

bjornthiberg force-pushed the 2024 branch from 509dfcd to d8ac9a1 Compare September 12, 2024 13:54

bjornthiberg added 6 commits September 12, 2024 16:23

use issue number instead of title

a0a9a86

use newest versions of checkout and setup-python, pin python 3.11.8

ab6b4e5

set timezone in script instead of workflow file

e8d3079

make default behavior be print in plaintext

a78f931

Move configuration to external JSON file

46064c3

fix: use correct file path for config

79ef6a4

bjornthiberg requested a review from algomaster99 September 12, 2024 15:31

algomaster99 requested changes Sep 12, 2024

View reviewed changes

contributions/open-source/bthiberg/README.md Outdated Show resolved Hide resolved

bjornthiberg added 3 commits September 12, 2024 23:15

remove task proposal

dcdb3fd

add required yearly updates to documentation

71c9c9f

separate commenting duration into lecture and leeway durations

ef44d53

bjornthiberg requested a review from algomaster99 September 12, 2024 21:36

algomaster99 requested changes Sep 12, 2024

View reviewed changes

update workflow to run in tools dir, get config from current dir

053b348

algomaster99 approved these changes Sep 12, 2024

View reviewed changes

fix: use timezone in "last changed" timestamps

5c90115

algomaster99 merged commit 7ca38a7 into KTH:2024 Sep 12, 2024
1 check passed

bjornthiberg mentioned this pull request Sep 17, 2024

Sort participation table by number of lectures attended #2498

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add lecture participation tracker using GitHub Actions (#2459) #2463

Add lecture participation tracker using GitHub Actions (#2459) #2463

bjornthiberg commented Sep 11, 2024 •

edited

Loading

algomaster99 left a comment

bjornthiberg commented Sep 12, 2024

bjornthiberg commented Sep 12, 2024

algomaster99 left a comment •

edited

Loading

algomaster99 Sep 12, 2024

bjornthiberg Sep 12, 2024

algomaster99 Sep 12, 2024

bjornthiberg Sep 12, 2024 •

edited

Loading

algomaster99 Sep 12, 2024

algomaster99 Sep 12, 2024

bjornthiberg Sep 12, 2024

algomaster99 Sep 12, 2024

bjornthiberg Sep 12, 2024

bjornthiberg commented Sep 12, 2024

bjornthiberg commented Sep 12, 2024

algomaster99 left a comment

algomaster99 Sep 12, 2024

algomaster99 commented Sep 12, 2024

bjornthiberg commented Sep 12, 2024 •

edited

Loading

algomaster99 commented Sep 12, 2024

algomaster99 commented Sep 12, 2024

algomaster99 commented Sep 12, 2024

algomaster99 commented Sep 12, 2024

bjornthiberg commented Sep 14, 2024

algomaster99 commented Sep 14, 2024

	config = json.load(open("tools/track_participation_config.json"))
	config = json.load(open("track_participation_config.json"))

Add lecture participation tracker using GitHub Actions (#2459) #2463

Add lecture participation tracker using GitHub Actions (#2459) #2463

Conversation

bjornthiberg commented Sep 11, 2024 • edited Loading

Description

algomaster99 left a comment

Choose a reason for hiding this comment

bjornthiberg commented Sep 12, 2024

bjornthiberg commented Sep 12, 2024

algomaster99 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bjornthiberg Sep 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bjornthiberg commented Sep 12, 2024

bjornthiberg commented Sep 12, 2024

algomaster99 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

algomaster99 commented Sep 12, 2024

bjornthiberg commented Sep 12, 2024 • edited Loading

algomaster99 commented Sep 12, 2024

algomaster99 commented Sep 12, 2024

algomaster99 commented Sep 12, 2024

algomaster99 commented Sep 12, 2024

bjornthiberg commented Sep 14, 2024

algomaster99 commented Sep 14, 2024

bjornthiberg commented Sep 11, 2024 •

edited

Loading

algomaster99 left a comment •

edited

Loading

bjornthiberg Sep 12, 2024 •

edited

Loading

bjornthiberg commented Sep 12, 2024 •

edited

Loading