Skip to content

Added twitter card functionality #196

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Added twitter card functionality #196

wants to merge 2 commits into from

Conversation

blackhat-7
Copy link

I have added the twitter card functionality. So now it extracts namespaces and properties of the twitter cards.
I have also added 3 test cases

This was a needed feature in issue #179

For example the following now works:

>>> extruct.extract('<!doctype html><html><head><meta name="twitter:card" content="summary">')
{'microdata': [], 
'json-ld': [], 
'opengraph': [], 
'microformat': [], 
'rdfa': [],
 'dublincore': [{'namespaces': {}, 'elements': [], 'terms': []}], 
'twittercard': [{'namespace': {'twitter': 'https://dev.twitter.com/cards#'}, 'properties': [('twitter:card', 'summary')]}]}

@blackhat-7 blackhat-7 changed the title Twittercard Added twitter card functionality May 13, 2022
@blackhat-7 blackhat-7 mentioned this pull request May 13, 2022
Open
@stephentgrammer
Copy link

Would love to see this get in the next version tag! @blackhat-7, you might want to update the REAME to mention twitter card usage.

lopuhin
lopuhin reviewed May 16, 2022
View reviewed changes
Copy link
Member

@lopuhin lopuhin 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 the PR @blackhat-7 , still reading a code, left one comment below.

tests/test_twittercard.py
from extruct.twittercard import TwitterCardExtractor
from tests import get_testdata, jsonize_dict

class TestTwittercard(unittest.TestCase):
Copy link
Member

Choose a reason for hiding this comment

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

Are these tests run? I don't see them in the log here https://github.com/scrapinghub/extruct/runs/6452050839?check_suite_focus=true#step:5:20

@naktinis
Copy link

Anything remaining to be done here? Just include the tests in the test suite and document in README?

# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@lopuhin lopuhin lopuhin left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@blackhat-7 @stephentgrammer @naktinis @lopuhin