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

PR2: Add new preference and AnnouncementChecker class #1509

Open
wants to merge 35 commits into
base: shrivaths/changelog-announcement-1
Choose a base branch
from
Open
Changes from 22 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
de22a57
Add new preference and annoucementchecker class
shrivaths16 Sep 19, 2023
69ff1ed
Merge branch 'shrivaths/changelog-announcement-1' into shrivaths/chan…
shrivaths16 Sep 20, 2023
3fc0e75
Merge branch 'shrivaths/changelog-announcement-1' into shrivaths/chan…
shrivaths16 Sep 21, 2023
0db2904
Add new preferences and bulletin json converter
shrivaths16 Sep 28, 2023
7ab6d8b
Merge branch 'shrivaths/changelog-announcement-1' into shrivaths/chan…
shrivaths16 Sep 29, 2023
7639142
Merge branch 'shrivaths/changelog-announcement-1' into shrivaths/chan…
shrivaths16 Oct 2, 2023
babb86d
Merge branch 'shrivaths/changelog-announcement-1' of https://github.c…
shrivaths16 Oct 3, 2023
6eddcf0
Add some of the suggestions
shrivaths16 Oct 3, 2023
3f99b8b
Typo
shrivaths16 Oct 3, 2023
f8fb1f1
Merge branch 'shrivaths/changelog-announcement-1' into shrivaths/chan…
shrivaths16 Oct 12, 2023
d9947c3
Add test function for the announcement checker class
shrivaths16 Oct 16, 2023
4cb0c6a
Fix path bug
shrivaths16 Oct 16, 2023
3323115
Add boolean for new announcement
shrivaths16 Oct 17, 2023
fc856bf
Add fixtures, pass mainwindow states
shrivaths16 Oct 18, 2023
39ed0b6
Lint files
shrivaths16 Oct 19, 2023
dce4dab
Fix bulletin json creation
shrivaths16 Oct 30, 2023
9e1ebf7
Set latest data with title
shrivaths16 Oct 30, 2023
d35f029
Correct json directory
shrivaths16 Nov 2, 2023
21ea789
additional condition to announcementchecker class
shrivaths16 Nov 30, 2023
3cc4188
Better date comparison, more tests
shrivaths16 Dec 2, 2023
59f07ae
Add more conditions to announcementchecker class
shrivaths16 Dec 5, 2023
acdf130
Correct default value in prefs
shrivaths16 Dec 5, 2023
fbb1039
Merge branch 'shrivaths/changelog-announcement-1' into shrivaths/chan…
shrivaths16 Dec 14, 2023
63e329a
Merge branch 'shrivaths/changelog-announcement-1' into shrivaths/chan…
shrivaths16 Dec 20, 2023
cbb661d
Merge branch 'shrivaths/changelog-announcement-1' into shrivaths/chan…
shrivaths16 Dec 21, 2023
9c726af
Merge branch 'shrivaths/changelog-announcement-1' into shrivaths/chan…
shrivaths16 Jan 3, 2024
68cb64b
add branch to website workflow
shrivaths16 Jan 6, 2024
2c92712
add json file to the website
shrivaths16 Jan 8, 2024
303a2d2
generate json in the correct path
shrivaths16 Jan 9, 2024
b708717
store json as text file
shrivaths16 Jan 9, 2024
a679e61
keep_files is set true
shrivaths16 Jan 9, 2024
02757e6
add json to _static directory
shrivaths16 Jan 9, 2024
090e36f
Modify AnnouncementChecker Class with bulletin url
shrivaths16 Jan 11, 2024
aca6c7a
add date to announcement
shrivaths16 Jan 11, 2024
c218b10
add regex for date check
shrivaths16 Jan 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions docs/make_bulletin_json.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import json
import os

# Set the file paths
input_md_file = os.path.join(os.path.dirname(__file__), "bulletin.md")
output_json_file = os.path.join(os.path.dirname(__file__), "bulletin.json")


def generate_json_file():
with open(input_md_file, "r", encoding="utf-8") as md_file:
markdown_content = md_file.read()
bulletin_json = []
content = ""

# Initialize title and date with default values
title = "DEFAULT_TITLE"
date = "DEFAULT_DATE"

for line in markdown_content.split("\n"):
# Skip if the line begins with #
if line.startswith("# "):
continue
elif line.startswith("---"):
bulletin_json.append({"title": title, "date": date, "content": content})
content = ""
# Reset title and date to their default values after each section
title = "DEFAULT_TITLE"
date = "DEFAULT_DATE"
elif line.startswith("## "):
title = line[3:].strip()
elif line.startswith("_"):
date = line[1 : len(line) - 1].strip()
Copy link
Collaborator

Choose a reason for hiding this comment

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

This probably needs a stronger check, like a date format regexp or another more explicit indicator. There are lots of cases where we might start a line with "_" in the main content.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added a regex that checks if the entire line has only the date content and it is in a certain format.

else:
content += line + "\n"
# Append last section
bulletin_json.append({"title": title, "date": date, "content": content})

with open(output_json_file, "w", encoding="utf-8") as json_file:
json.dump(bulletin_json, json_file, ensure_ascii=False, indent=4)

Copy link

Choose a reason for hiding this comment

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

The generate_json_file() function reads a Markdown file, parses its content, and writes the parsed data to a JSON file. The function is well-structured and follows the single responsibility principle. However, it's recommended to move the file paths inside the function or pass them as arguments to make the function more reusable.

- input_md_file = os.path.join(os.path.dirname(__file__), "bulletin.md")
- output_json_file = os.path.join(os.path.dirname(__file__), "bulletin.json")

def generate_json_file(input_md_file, output_json_file):
    # rest of the code

if __name__ == "__main__":
-    generate_json_file()
+    input_md_file = os.path.join(os.path.dirname(__file__), "bulletin.md")
+    output_json_file = os.path.join(os.path.dirname(__file__), "bulletin.json")
+    generate_json_file(input_md_file, output_json_file)

Commitable suggestion (Beta)
Suggested change
def generate_json_file():
with open(input_md_file, "r", encoding="utf-8") as md_file:
markdown_content = md_file.read()
bulletin_json = []
content = ""
# Initialize title and date with default values
title = "DEFAULT_TITLE"
date = "DEFAULT_DATE"
for line in markdown_content.split("\n"):
# Skip if the line begins with #
if line.startswith("# "):
continue
elif line.startswith("---"):
bulletin_json.append({"title": title, "date": date, "content": content})
content = ""
# Reset title and date to their default values after each section
title = "DEFAULT_TITLE"
date = "DEFAULT_DATE"
elif line.startswith("## "):
title = line[3:].strip()
elif line.startswith("_"):
date = line[1 : len(line) - 1].strip()
else:
content += line + "\n"
# Append last section
bulletin_json.append({"title": title, "date": date, "content": content})
with open(output_json_file, "w", encoding="utf-8") as json_file:
json.dump(bulletin_json, json_file, ensure_ascii=False, indent=4)
def generate_json_file(input_md_file, output_json_file):
with open(input_md_file, "r", encoding="utf-8") as md_file:
markdown_content = md_file.read()
bulletin_json = []
content = ""
# Initialize title and date with default values
title = "DEFAULT_TITLE"
date = "DEFAULT_DATE"
for line in markdown_content.split("\n"):
# Skip if the line begins with #
if line.startswith("# "):
continue
elif line.startswith("---"):
bulletin_json.append({"title": title, "date": date, "content": content})
content = ""
# Reset title and date to their default values after each section
title = "DEFAULT_TITLE"
date = "DEFAULT_DATE"
elif line.startswith("## "):
title = line[3:].strip()
elif line.startswith("_"):
date = line[1 : len(line) - 1].strip()
else:
content += line + "\n"
# Append last section
bulletin_json.append({"title": title, "date": date, "content": content})
with open(output_json_file, "w", encoding="utf-8") as json_file:
json.dump(bulletin_json, json_file, ensure_ascii=False, indent=4)
if __name__ == "__main__":
input_md_file = os.path.join(os.path.dirname(__file__), "bulletin.md")
output_json_file = os.path.join(os.path.dirname(__file__), "bulletin.json")
generate_json_file(input_md_file, output_json_file)


if __name__ == "__main__":
generate_json_file()
7 changes: 6 additions & 1 deletion sleap/gui/app.py
Original file line number Diff line number Diff line change
@@ -69,7 +69,7 @@
from sleap.gui.overlays.tracks import TrackListOverlay, TrackTrailOverlay
from sleap.gui.shortcuts import Shortcuts
from sleap.gui.state import GuiState
from sleap.gui.web import ReleaseChecker, ping_analytics
from sleap.gui.web import ReleaseChecker, AnnouncementChecker, ping_analytics
from sleap.gui.widgets.docks import (
InstancesDock,
SkeletonDock,
@@ -158,6 +158,8 @@ def __init__(
self.state["share usage data"] = prefs["share usage data"]
self.state["skeleton_preview_image"] = None
self.state["skeleton_description"] = "No skeleton loaded yet"
self.state["announcement last seen date"] = prefs["announcement last seen date"]
self.state["announcement"] = prefs["announcement"]
if no_usage_data:
self.state["share usage data"] = False
self.state["clipboard_track"] = None
@@ -168,6 +170,7 @@ def __init__(
self.state.connect("show non-visible nodes", self.plotFrame)

self.release_checker = ReleaseChecker()
self.announcement_checker = AnnouncementChecker(state=self.state)

if self.state["share usage data"]:
ping_analytics()
@@ -223,6 +226,8 @@ def closeEvent(self, event):
prefs["color predicted"] = self.state["color predicted"]
prefs["trail shade"] = self.state["trail_shade"]
prefs["share usage data"] = self.state["share usage data"]
prefs["announcement last seen date"] = self.state["announcement last seen date"]
prefs["announcement"] = self.state["announcement"]

# Save preferences.
prefs.save()
65 changes: 64 additions & 1 deletion sleap/gui/web.py
Original file line number Diff line number Diff line change
@@ -3,11 +3,16 @@
import attr
import pandas as pd
import requests
from typing import List, Dict, Any
from typing import List, Dict, Any, Optional, Tuple
import json
import os
from datetime import datetime


REPO_ID = "talmolab/sleap"
ANALYTICS_ENDPOINT = "https://analytics.sleap.ai/ping"
BASE_DIR = os.path.dirname(os.path.abspath(os.path.join(__file__, os.path.pardir)))
BULLETIN_JSON = os.path.join(BASE_DIR, "..", "docs", "bulletin.json")


@attr.s(auto_attribs=True)
@@ -146,6 +151,64 @@ def get_release(self, version: str) -> Release:
)


@attr.s(auto_attribs=True)
class AnnouncementChecker:
"""Checker for new announcements on the bulletin page of sleap."""

state: "GuiState"
bulletin_json_path: str = BULLETIN_JSON
_previous_announcement_date: str = None
_latest_data: Optional[Dict[str, str]] = None

@property
def previous_announcement_date(self):
_previous_announcement_date = self.state["announcement last seen date"]
return _previous_announcement_date
Comment on lines +165 to +167
Copy link

Choose a reason for hiding this comment

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

Consider handling the case where the key "announcement last seen date" might not exist in self.state to prevent a KeyError.


def _read_bulletin_data(self) -> Optional[Dict]:
"""Reads the bulletin data from the JSON file."""
try:
with open(self.bulletin_json_path, "r") as jsf:
data = json.load(jsf)
self._latest_data = data[0]
except FileNotFoundError:
self._latest_data = None

def new_announcement_available(self) -> bool:
"""Check if latest announcement is available."""
self._read_bulletin_data()
if self.previous_announcement_date and self._latest_data:
latest_date = datetime.strptime(self._latest_data["date"], "%m/%d/%Y")
previous_date = datetime.strptime(
self.previous_announcement_date, "%m/%d/%Y"
)
if latest_date > previous_date:
return True
else:
return False
else:
return True

def get_latest_announcement(self) -> Optional[Tuple[str, str, str]]:
"""Return latest announcements on the releases page not seen by user."""
if self.new_announcement_available():
return (
self._latest_data["title"],
self._latest_data["date"],
self._latest_data["content"],
)
return None

def update_announcement(self):
"""Update the last seen date of announcement in preferences."""
announcement = self.get_latest_announcement()
if announcement is None:
return
self.state["announcement last seen date"] = announcement[1]
new_announcement = "\n".join(announcement[2].split("\n"))
self.state["announcement"] = "## " + announcement[0] + "\n" + new_announcement


def get_analytics_data() -> Dict[str, Any]:
"""Gather data to be transmitted to analytics backend."""
import os
3 changes: 3 additions & 0 deletions sleap/prefs.py
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@
"""

from sleap import util
from datetime import date


class Preferences(object):
@@ -28,6 +29,8 @@ class Preferences(object):
"node label size": 12,
"show non-visible nodes": True,
"share usage data": True,
"announcement last seen date": False,
Copy link

@coderabbitai coderabbitai bot Dec 5, 2023

Choose a reason for hiding this comment

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

The default value for "announcement last seen date" is set to False, which is not a date type. Consider using None or a specific date format if the intention is to store a date.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed, do this.

Copy link

Choose a reason for hiding this comment

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

@talmo I've updated the default value for "announcement last seen date" to None to reflect that it should store a date type. Here's the updated code snippet:

_defaults = {
    # ... other defaults ...
    "announcement last seen date": None,
    # ... other defaults ...
}

Please ensure to handle this None value appropriately in the code where the announcement last seen date preference is accessed or updated.

"announcement": False,
Copy link

@coderabbitai coderabbitai bot Dec 5, 2023

Choose a reason for hiding this comment

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

The default value for "announcement" is set to False. If this preference is intended to store a string, consider using an empty string "" or None as a more appropriate default value.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed, do this.

Copy link

Choose a reason for hiding this comment

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

@talmo I've noted the change for the default value of "announcement" to be an empty string or None. Since I cannot commit code, here's the suggested update for the Preferences class defaults:

_defaults = {
    # ... other defaults ...
    "announcement last seen date": None,
    "announcement": "",
}

You can apply this change to the prefs.py file. If you need any further assistance, feel free to ask!

}
_filename = "preferences.yaml"

1 change: 1 addition & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
@@ -11,3 +11,4 @@
from tests.fixtures.datasets import *
from tests.fixtures.videos import *
from tests.fixtures.models import *
from tests.fixtures.announcements import *
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[{"title": "title1", "date": "10/09/2023", "content": "New announcement"}, {"title": "title2", "date": "10/07/2023", "content": "Old Announcment"}]
8 changes: 8 additions & 0 deletions tests/fixtures/announcements.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import pytest

TEST_BULLETIN_JSON = "tests/data/announcement_checker_bulletin/test_bulletin.json"


@pytest.fixture
def bulletin_json_path():
return TEST_BULLETIN_JSON
55 changes: 54 additions & 1 deletion tests/gui/test_web.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
import pandas as pd
from sleap.gui.web import ReleaseChecker, Release, get_analytics_data, ping_analytics
from sleap.gui.web import (
ReleaseChecker,
Release,
AnnouncementChecker,
get_analytics_data,
ping_analytics,
)
import pytest
import json
import os
from sleap.gui.commands import CommandContext
from sleap.io.dataset import Labels


def test_release_from_json():
@@ -72,6 +82,49 @@ def test_release_checker():
assert checker.releases[1] != rls_test


def test_announcementchecker(bulletin_json_path):

labels = Labels()
context = CommandContext.from_labels(labels=labels)
context.state = {}
context.state["announcement last seen date"] = "10/10/2023"
checker = AnnouncementChecker(
state=context.state, bulletin_json_path=bulletin_json_path
)

# Check if the announcement checker gets the correct date from the app
assert checker.previous_announcement_date == "10/10/2023"

# Create dummy JSON file to check
bulletin_data = [
{"title": "title1", "date": "10/12/2023", "content": "New announcement"},
{"title": "title2", "date": "10/07/2023", "content": "Old Announcment"},
]
with open(bulletin_json_path, "w") as test_file:
json.dump(bulletin_data, test_file)

# Check if latest announcement is fetched
announcement = checker.get_latest_announcement()
assert announcement == ("title1", "10/12/2023", "New announcement")

# Check if announcement is updated
checker.update_announcement()
assert context.state["announcement last seen date"] == "10/12/2023"
assert context.state["announcement"] == "## title1\nNew announcement"

# Create dummy JSON file
bulletin_data = [
{"title": "title1", "date": "10/09/2023", "content": "New announcement"},
{"title": "title2", "date": "10/07/2023", "content": "Old Announcment"},
]
with open(bulletin_json_path, "w") as test_file:
json.dump(bulletin_data, test_file)

# Check to ensure no new announcement is created
announcement = checker.get_latest_announcement()
assert announcement == None


This comment was marked as off-topic.

def test_get_analytics_data():
analytics_data = get_analytics_data()
assert "platform" in analytics_data
Loading