Skip to content

Conversation

@sagivharel
Copy link
Collaborator

@atalyaalon I added the solution for the bug I came across a few weeks ago.

sagivharel and others added 5 commits March 15, 2020 14:36
pr from anyway-hasadna
added a new column in news_flash table for tags
added a function which create tags based on flash content
@sagivharel sagivharel changed the title Updated Docker.md file with a small insight Updated Docker.md file with a small insight + created function for tags creation for news flashs Mar 16, 2020
@sagivharel
Copy link
Collaborator Author

@atalyaalon I have created the function and also did the required modification to the table.
@Mano3 I didn't want to mess up your code so I will leave the function implementation for the ynet scrapper to you. Just need to call my function and send it the desired text. (I recommend send it the title and description concatenated together for the best results)

@sagivharel
Copy link
Collaborator Author

CI failure caused by not sending values for parameter tags in ynet scraper function

@atalyaalon
Copy link
Collaborator

atalyaalon commented Mar 20, 2020

@sagivharel wonderful! great work!

A few notes:

  1. Sorry - before I missed your messages - regarding the call of the function insert_new_flash_news in ynet_spider please consult @Mano3 and add this fix to this pr.
  2. I think it's better to add either: boolean columns for the tags OR an array field in postgres. However, let's leave it like this for now and perhaps change it in the future.
  3. We need to add tags of: תאונה חזיתית, התהפכות, תאונה עם הולך רגל, תאונה עם ילד/ה או פעוט/ה, תאונה עם קשיש/ה או אדם/אישה מבוגר/ת
    All of the above is an important information we don't want to miss.
    @Mano3 any comments?

@Mano3
Copy link
Collaborator

Mano3 commented Mar 20, 2020

@sagivharel
Seems excellent overall.
I agree with Atalya about the necessity of age tags and accident type tags.
About adding the function to ynet and walla, I rather you insert it yourself so you would get to know the code better, talk with me while you try and I will help.

@atalyaalon atalyaalon linked an issue Apr 1, 2020 that may be closed by this pull request
@atalyaalon atalyaalon requested a review from Mano3 April 18, 2020 15:48
@atalyaalon
Copy link
Collaborator

@Mano3 just to make sure - you're gonna continue this one right?

@Mano3
Copy link
Collaborator

Mano3 commented Apr 23, 2020

@atalyaalon If that's urgent I rather someone else would finish this at the moment.

@Mano3
Copy link
Collaborator

Mano3 commented Jun 5, 2020

@advak Hi Adva, any progress on this?
Quite a lot of changes were made into news flash scraping code. I suggest talking with @elazarg before any update is made

@elazarg
Copy link

elazarg commented Jun 5, 2020

I think for now we can merge only news_flash_tags.py, and when the refactoring is over, adding tags should be a one liner (and of course upgrade/downgrade)

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create tags for news flashes

7 participants