Hey,
ich habe einen Crawler geschrieben. Dieser funktioniert auch, jedoch bin ich ziemlich neu in Python und wäre froh, wenn mir jemand Verbesserungsvorschläge geben könnte.
Hier ist der Crawler: https://github.com/ford42/CrawlerHeise
Vielen Dank
LG ford
Kann bitte jemand über meinen Python-Crawler schauen?
- sls
- User
- Beiträge: 480
- Registriert: Mittwoch 13. Mai 2015, 23:52
- Wohnort: Country country = new Zealand();
Sollen wir dabei auch auf die Issues / Code-Reviews direkt auf Github eingehen, die teilweise ja auch nicht so ganz richtig sind? Ich kenne von Python nur `requirements.txt` welche idealerweise mit pip freeze erstellt wird. while True: if -> break ist valides Python und sogar die bevorzugte Variante, warum, tonnenweise hier im Forum zu lesen.
Bevor man sich das anschaut, hat das einen Grund dass du IDE-relevante Dateien (.idea) in dein Repo eincheckst? Da solltest du mal lieber dein .gitignore pflegen, das wollen andere bei einem git clone vielleicht nicht mitziehen
Bevor man sich das anschaut, hat das einen Grund dass du IDE-relevante Dateien (.idea) in dein Repo eincheckst? Da solltest du mal lieber dein .gitignore pflegen, das wollen andere bei einem git clone vielleicht nicht mitziehen
When we say computer, we mean the electronic computer.
Konstanten schreibt man groß: also URL statt url. Wenn CrawledArticle NamedTuple wären, wäre das writerow auch deutlich einfacher.
Eine Klasse pro Datei ist in Python etwas übertrieben. Also könnte man auch alles in __init__.py schreiben, und weil das dann die einzige Datei im Paket wäre, könnte man es zu einem Modul machen.
In ArticleFetcher wird url_parsed als Attribut definiert aber nie benutzt; das kann eine lokale Variable sein; oder man kann es gleich weglassen, denn ob da ›seite=1‹ in der Url steht oder nicht, ist auch egal.
In ›next_page‹ und ›fetch‹ wird jeweils die Seite gelesen und geparst. Das ist einmal zu viel. ›next_page‹ erhält außerdem als Parameter url, was aber immer self.url ist, also ist der Parameter überflüssig. ›next_page‹ würde auch suggerieren, dass die nächste Seite gelesen wird, aber es wird nur geprüft, ob es eine nächste Seite gibt, ›has_next_page‹ wäre also ein besserer Name.
In ›fetch‹, was soll das sleep? Die Funktion ist etwas lang, und sollte wenigstens die innere for-Schleife als Funktion ›parse_article‹ auslagern.
Die vielen if-Abfragen sind alle identisch und sollten durch einen Funktionsaufruf ersetzt werden.
Das Exception-Handling ist völlig ungeeignet. Exceptions immer so konkret wie möglich abfangen. Die meisten Fehler, die auftreten könnten, werden wohl nichts damit zu tun haben, dass die URL falsch ist. Mit der Aussage kann man also den Fehler nie finden. ›exit‹ hat in einem normalen Programm nichts verloren, weil auf diese Weise kann der Aufrufer nichts tun, um den Fehlerfall besser zu verarbeiten. Die beste Fehlerbehandlung ist daher, sie gar nicht zu behandeln.
Da die Klasse eigentlich nur aus einem fetch besteht, handelt es sich nur um eine Funktion.
Die Datei ›crawler.py‹ könnte also so aussehen:
und das Hauptprogramm:
EDIT: es war nicht gemeint, für jeden Artikel eine neue Klasse zu erstellen.
Eine Klasse pro Datei ist in Python etwas übertrieben. Also könnte man auch alles in __init__.py schreiben, und weil das dann die einzige Datei im Paket wäre, könnte man es zu einem Modul machen.
In ArticleFetcher wird url_parsed als Attribut definiert aber nie benutzt; das kann eine lokale Variable sein; oder man kann es gleich weglassen, denn ob da ›seite=1‹ in der Url steht oder nicht, ist auch egal.
In ›next_page‹ und ›fetch‹ wird jeweils die Seite gelesen und geparst. Das ist einmal zu viel. ›next_page‹ erhält außerdem als Parameter url, was aber immer self.url ist, also ist der Parameter überflüssig. ›next_page‹ würde auch suggerieren, dass die nächste Seite gelesen wird, aber es wird nur geprüft, ob es eine nächste Seite gibt, ›has_next_page‹ wäre also ein besserer Name.
In ›fetch‹, was soll das sleep? Die Funktion ist etwas lang, und sollte wenigstens die innere for-Schleife als Funktion ›parse_article‹ auslagern.
Die vielen if-Abfragen sind alle identisch und sollten durch einen Funktionsaufruf ersetzt werden.
Das Exception-Handling ist völlig ungeeignet. Exceptions immer so konkret wie möglich abfangen. Die meisten Fehler, die auftreten könnten, werden wohl nichts damit zu tun haben, dass die URL falsch ist. Mit der Aussage kann man also den Fehler nie finden. ›exit‹ hat in einem normalen Programm nichts verloren, weil auf diese Weise kann der Aufrufer nichts tun, um den Fehlerfall besser zu verarbeiten. Die beste Fehlerbehandlung ist daher, sie gar nicht zu behandeln.
Da die Klasse eigentlich nur aus einem fetch besteht, handelt es sich nur um eine Funktion.
Die Datei ›crawler.py‹ könnte also so aussehen:
Code: Alles auswählen
from collections import namedtuple
import requests
from bs4 import BeautifulSoup
from urllib.parse import urljoin, urlparse
CrawledArticle = namedtuple("CrawledArticle", "title, content, image, author, data, count_comments")
def get_text(post, selector, default):
element = post.select_one(selector)
return default if element is None else element.text
def parse_article(url, post):
title = get_text(post, ".tp_title", "Kein Title vorhanden!")
content = get_text(post, "p", "Kein Content vorhanden!")
author = get_text(post, "li.has-author", "Kein Autor vorhanden!")
data = get_text(post, "time", "Kein Datum vorhanden!")
count_comments = get_text(post, "span.count.comment_count", "Keine Anzahl der Kommentare vorhanden!")
img = post.select_one("img")
image = urljoin(url, img.attrs["src"]) if img is not None else "Kein Bild vorhanden!"
return CrawledArticle(title, content, image, author, data, count_comments)
def get_next_url(url, doc):
url_sub = doc.select_one(".seite_weiter")
if url_sub is not None:
return urljoin(url, url_sub.attrs["href"])
return None
def fetch_articles(url):
articles = []
while url:
r = requests.get(url)
doc = BeautifulSoup(r.text, "html.parser")
for post in doc.select("article.news.row"):
articles.append(parse_article(url, post))
url = get_next_url(url, doc)
return articles
Code: Alles auswählen
import csv
import crawler
URL = "https://www.heise.de/tp/energie-und-klima/?seite=1"
def write_to_csv(articles):
with open('articlesHeisse.csv', 'w', newline='') as csv_file:
article_writer = csv.writer(csv_file, delimiter=';', quotechar='"', quoting=csv.QUOTE_MINIMAL)
for article in articles:
article_writer.writerow(article)
def main():
write_to_csv(fetch_articles(url))
if __name__ == '__main__':
main()
Ich würde auch noch aus `fetch_articles` einen Generator machen und die `writerows`-Methode des Writers benutzen.
Außerdem könnte man `get_next_url` auch in einen Iterator abstrahieren, der über die Seiten iteriert, und damit das Zurückgeben von `None` vermeiden (ungetestet):
Ob das jetzt unbedingt besser ist, darüber lässt sich streiten.
Außerdem könnte man `get_next_url` auch in einen Iterator abstrahieren, der über die Seiten iteriert, und damit das Zurückgeben von `None` vermeiden (ungetestet):
Code: Alles auswählen
def iter_pages(url):
while True:
doc = BeautifulSoup(requests.get(url).text, "html.parser")
yield url, doc
url_sub = doc.select_one(".seite_weiter")
if url_sub is None:
return
url = urljoin(url, url_sub.attrs["href"])
def fetch_articles(url):
for page_url, page in iter_pages(url):
for post in page.select("article.news.row"):
yield parse_article(page_url, post)