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

PoC/discussion: add links() #216

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

noviluni
Copy link
Member

@noviluni noviluni commented May 4, 2021

This is just an idea I had that I would like to discuss.

I've seen that the requests-html library adds a links() method to extract all the URLs in the HTML. In parsel you can easily extract the links by using css() or xpath(), but there could be some logic that could be simplified by creating a links() method (see code).

A possible use would be to use it in conjunction with scrapy.Response.follow_all() to easily crawl all URLs in the HTML.

Of course, this should be refactored, properly documented, and tested. Or even converted to a generator. We could also exclude other URL patterns like tel: or even add a method parameter to filter the URLS (by a regex pattern like we do in scrapy.sitemap.SitemapSpider?) or implement an option to get the absolute links.

So I would like to know what you think about this, if you think it's useful or not and if it should be implemented or not (or alternatively, if it should be implemented directly into the scrapy.selector.Selector) 🙂

Resolves #26

@codecov
Copy link

codecov bot commented May 4, 2021

Codecov Report

Merging #216 (9663dd1) into master (80509e4) will decrease coverage by 0.33%.
The diff coverage is 50.00%.

❗ Current head 9663dd1 differs from pull request most recent head f0a8c56. Consider uploading reports for the commit f0a8c56 to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##            master     #216      +/-   ##
===========================================
- Coverage   100.00%   99.66%   -0.34%     
===========================================
  Files            5        5              
  Lines          298      300       +2     
  Branches        51       52       +1     
===========================================
+ Hits           298      299       +1     
- Misses           0        1       +1     
Impacted Files Coverage Δ
parsel/selector.py 99.36% <50.00%> (-0.64%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 80509e4...f0a8c56. Read the comment docs.

@Gallaecio
Copy link
Member

See #26 for some background discussion that goes back to a Scrapy issue.

@noviluni noviluni changed the title PoC/discussin: add links() PoC/discussion: add links() Aug 8, 2021
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.

[From Scrapy] Selector.extract_links and links
2 participants