-
Notifications
You must be signed in to change notification settings - Fork 99
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
base: shrivaths/changelog-announcement-1
Are you sure you want to change the base?
Changes from 9 commits
de22a57
69ff1ed
3fc0e75
0db2904
7ab6d8b
7639142
babb86d
6eddcf0
3f99b8b
f8fb1f1
d9947c3
4cb0c6a
3323115
fc856bf
39ed0b6
dce4dab
9e1ebf7
d35f029
21ea789
3cc4188
59f07ae
acdf130
fbb1039
63e329a
cbb661d
9c726af
68cb64b
2c92712
303a2d2
b708717
a679e61
02757e6
090e36f
aca6c7a
c218b10
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
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'): | ||
if 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" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The script assumes that each section in the markdown file starts with |
||
elif line.startswith('##'): | ||
title = line[3:].strip() | ||
elif line.startswith('_'): | ||
date = line[1:len(line)-1].strip() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The date extraction logic assumes that the date is surrounded by underscores ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We plan on sticking with this format, but perhaps we should add a test that the current bulletin.md changes can be parsed correctly. I'm curious, can you give an example of how you would parse the date? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @roomrys: Sure, I can provide an example using regular expressions which is a more robust way to parse the date. Here's how you could modify your code: import re
# ...
for line in markdown_content.split('\n'):
if line.startswith('---'):
bulletin_json.append({'title': title, 'date': date, 'content':content})
content = ''
elif line.startswith('##'):
title = line[3:].strip()
else:
match = re.search(r'_([^_]+)_', line)
if match:
date = match.group(1).strip()
else:
content += (line + '\n') In this code, For testing, you could create a separate function that takes a string as input and returns the parsed title, date, and content. Then, you can write tests that call this function with different inputs and check that the output is as expected. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Basically the same logic, but with regex which is more difficult to understand imo. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @roomrys: I understand your concern. Regular expressions can indeed be a bit difficult to grasp initially, but they offer a lot of flexibility and robustness when it comes to text parsing. In this case, the regular expression is used to ensure that the date extraction works correctly even if there are additional underscores in the line. However, if you prefer to stick with the current approach, it's important to make sure that the format of the dates in the markdown file remains consistent. Any changes could potentially break the date extraction logic. As you suggested earlier, adding tests to verify that the parsing works correctly would be a good way to catch any issues early on. If you have any other questions or need further clarification, feel free to ask! |
||
else: | ||
content += (line + '\n') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The script assumes that the title of a section starts with |
||
# Append last section | ||
bulletin_json.append({'title': title, 'date': date, 'content':content}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The variables - content = ''
+ title = date = content = '' There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pretty easy preventative measure. Maybe we can set defaults and raise a warning if the parser never overrides the default value? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @roomrys, that's a great suggestion. We can initialize import json
import warnings
# Set the file paths
input_md_file = 'D:\TalmoLab\sleap\docs\bulletin.md'
output_json_file = 'D:\TalmoLab\sleap\docs\\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'):
if 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})
# Check if title or date were never updated from their default values
if title == "DEFAULT_TITLE" or date == "DEFAULT_DATE":
warnings.warn("The parser did not find a title or date in the markdown file.")
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__':
generate_json_file() This way, we can ensure that the script doesn't fail due to uninitialized variables and also alert the user if the markdown file is not formatted as expected. |
||
|
||
with open(output_json_file, 'w', encoding='utf-8') as json_file: | ||
json.dump(bulletin_json, json_file, ensure_ascii=False, indent=4) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The script writes the generated JSON data to a file without checking if the file already exists. If the file does exist, its contents will be overwritten. Consider checking if the file exists before writing to it and prompt the user for confirmation if it does. + if os.path.exists(output_json_file):
+ print(f"File {output_json_file} already exists. Overwrite? (y/n)")
+ response = input().strip().lower()
+ if response != 'y':
+ return
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__': | ||
generate_json_file() |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,11 +3,15 @@ | |
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 | ||
|
||
|
||
REPO_ID = "talmolab/sleap" | ||
ANALYTICS_ENDPOINT = "https://analytics.sleap.ai/ping" | ||
BASE_DIR = os.path.dirname(os.path.abspath(__file__)) | ||
BULLETIN_JSON = os.path.join(BASE_DIR, "..", "docs", "bulletin.json") | ||
|
||
|
||
@attr.s(auto_attribs=True) | ||
|
@@ -146,6 +150,40 @@ def get_release(self, version: str) -> Release: | |
) | ||
|
||
|
||
@attr.s(auto_attribs=True) | ||
class AnnouncementChecker: | ||
"""Checker for new announcements on the bulletin page of sleap.""" | ||
|
||
app: "MainWindow" | ||
shrivaths16 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
bulletin_json_path: str = BULLETIN_JSON | ||
previous_announcement_date: str = None | ||
_latest_data: Optional[Dict[str, str]] = None | ||
|
||
def _read_bulletin_data(self) -> Dict[str, str]: | ||
"""Reads the bulletin data from the JSON file.""" | ||
try: | ||
with open(self.bulletin_json_path, 'r', encoding='utf-8') as jsf: | ||
data = json.load(jsf) | ||
return data[0] | ||
except FileNotFoundError: | ||
return {} | ||
|
||
def get_latest_announcement(self) -> Optional[Tuple[str, str]]: | ||
"""Return latest announcements on the releases page not seen by user.""" | ||
self._latest_data = self._read_bulletin_data() | ||
if self._latest_data and self._latest_data['date'] != self.previous_announcement_date: | ||
return (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.app.state["announcement last seen date"] = announcement[0] | ||
self.app.state["announcement"] = announcement[1] | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A new class However, there's a potential issue with the Here's a suggested fix: self._latest_data = self._read_bulletin_data()
- if self._latest_data and self._latest_data['date'] != self.previous_announcement_date:
+ if self._latest_data and 'date' in self._latest_data and self._latest_data['date'] != self.previous_announcement_date:
return (self._latest_data['date'], self._latest_data['content'])
return None |
||
|
||
def get_analytics_data() -> Dict[str, Any]: | ||
"""Gather data to be transmitted to analytics backend.""" | ||
import os | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The script uses absolute paths to the markdown and JSON files. This could lead to issues if the script is run from a different directory or on a different machine where the file paths may not be the same. Consider using relative paths instead.