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 NationalPost #584

Merged
merged 4 commits into from
Sep 3, 2024
Merged

Add NationalPost #584

merged 4 commits into from
Sep 3, 2024

Conversation

addie9800
Copy link
Collaborator

No description provided.

@addie9800 addie9800 changed the title Add ˋNationalPostˋ Add NationalPost Aug 12, 2024
Copy link
Collaborator

@MaxDall MaxDall left a comment

Choose a reason for hiding this comment

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

Thanks for adding 👍

Comment on lines 53 to 61
filter_list = ["Curated", "News", "Newsroom daily", "story", "Canada", "World"]
filtered_topics = [
topic
for topic in preliminary_topics
if "NLP Entity Tokens" not in topic
and "NLP Category" not in topic
and topic not in filter_list
and not re.search(r"[0-9a-f]{8}-([0-9a-f]{4}-){3}[0-9a-f]{12}", topic)
]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe some sort of regex filter would be nice here. Actually, I think you could just use the regular regex_filter.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I do agree. Using regex_filter seems a bit cursed Typing wise, but it does work just fine.

src/fundus/publishers/ca/__init__.py Outdated Show resolved Hide resolved
Comment on lines 2 to 11
"V1": {
"authors": [
"Special to National Post"
],
"body": {
"summary": [
"A draft policy by the CMA essentially calls for an end to private workplace coverage for virtual health care"
],
"sections": [
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it would be good to add a test case including at least one subheadline.

@addie9800 addie9800 requested a review from MaxDall August 28, 2024 11:19
MaxDall
MaxDall previously approved these changes Sep 3, 2024
Copy link
Collaborator

@MaxDall MaxDall left a comment

Choose a reason for hiding this comment

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

Thanks for adding! Looks good 👍

Base automatically changed from add-canadian-news-sources to master September 3, 2024 11:16
@addie9800 addie9800 dismissed MaxDall’s stale review September 3, 2024 11:16

The base branch was changed.

@MaxDall MaxDall merged commit 34a7f30 into master Sep 3, 2024
5 checks passed
@MaxDall MaxDall deleted the add-national-post branch September 3, 2024 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants