ProgrammeringsBaren! Småprat, om det du elsker!

[Lycantrophe]

PhelpsTransposed vet du ikke at det og bruke regex til å parse HTML er noe man ikke bør gjøre?

Python har veldig gode parsere i BeautifulSoup og lxml.

Lese denne posten.

Du har åpenbart forstått innholdet.

 

Eller nei. Det å matche noen deler av et html-doc med regex er helt greit.

[PhelpsTransposed]

PhelpsTransposed vet du ikke at det og bruke regex til å parse HTML er noe man ikke bør gjøre?

Python har veldig gode parsere i BeautifulSoup og lxml.

Lese denne posten.

Niks!

 

 

Haha ja, vurderer om ikke det var

 if not any(new_url in word for word in urls_Crawled) and not any(new_url in word for word in urlsToCrawl_Parent)and not any(new_url in word for word in urlsToCrawl_Child) and '<' not in new_url and '#' not in new_url and any(word in new_url.split('.')[-1] for word in allowedList) and 'https://wow' not in new_url and 'mailto' not in new_url:

som gjorde at de fikk skrekken for meg. Den var utrolig stygg. Som sagt, gjort på en dag og gjort for å få lov til å komme på intervju. Man lærer så lenge man lever ;D

 

Det er ikke bare den. Det er mange tvilsomme valg tatt, skal jeg være ærlig.

 

Flott å være ærlig Det er absolutt ikke noe kode jeg er stolt av på noen som helst måte, eller dvs jeg er stolt av å ha laget noe som fungerer på en dag, men når det gjelder hele greia så er det nok mye selv jeg kunne ha fikset på, og som jeg har planer om å fikse på så snart eksamenstida er over.

 

Try/Catchen kastet jeg på etter at jeg kjørte en live test på 100 000 url'er, og så at det var mye rare enkelt-ting som ødelagte crawlinga.

Endret 11. mai 2014 av PhelpsTransposed

[Lycantrophe]

Jevnt over er det største problemet mitt struktureringen din.

 

Nå gjør du:

 

def crawler(url):
    # The crawler find links by utilizing pythons urllib and the href tag
    for new_url in re.findall('''href=["'](.[^"']+)["']''', urllib.urlopen(url).read()):
        new_url = re.sub('\/*$', '', new_url)

        # A bit ugly, but this is to be sure that the new url is not in the urls already crawled, or scheduled for crawling, a check for if it's a HTML element that have escape the regex, or a "link" to a html class on the same page, and that the url ending is in the allowed list
        if not any(new_url in word for word in urls_Crawled) and not any(new_url in word for word in urlsToCrawl_Parent)and not any(new_url in word for word in urlsToCrawl_Child) and '<' not in new_url and '#' not in new_url and any(word in new_url.split('.')[-1] for word in allowedList) and 'https://wow' not in new_url and 'mailto' not in new_url:

            # If the url is in the new url, f.ex http://www.telenor.com is inside http://www.telenor.com/no/personvern/feed
            # the latter should be appended in the parent list as i prioritize the local urls first.
            if url.replace("http://","").replace("www.","").split('/', 1)[0] in new_url:
                urlsToCrawl_Parent.append(new_url)

            # Another check, this is if we deal with a local link on the form /no/personvern/feed/
            elif new_url.startswith('/'):
                # To better store local links, f.ex /no/personvern/feed/ forward slash (/) is omitted
                new_url = re.sub('\/*$', '', new_url)
                url = re.sub('\/*$', '', url)

                # The /no/personvern/feed/ url is stored as www.telenor.com/no/personvern/feed/
                urlsToCrawl_Parent.append(url+new_url)

            # If we are not dealing with an local URL we are dealing with a child URL
            else:
                urlsToCrawl_Child.append(new_url)

Det jeg synes du burde gjort er:

 

def urls(url):
    urlsToCrawl_Parent = []
    urlsToCrawl_Child = []

    # The crawler find links by utilizing pythons urllib and the href tag
    for new_url in re.findall('''href=["'](.[^"']+)["']''', urllib.urlopen(url).read()):
        new_url = re.sub('\/*$', '', new_url)

        if any(new_url in word for word in urls_Crawled) and not any(new_url in word for word in urlsToCrawl_Parent)and not any(new_url in word for word in urlsToCrawl_Child) and '<' not in new_url and '#' not in new_url and any(word in new_url.split('.')[-1] for word in allowedList) and 'https://wow' not in new_url and 'mailto' not in new_url:
            continue

        if url.replace("http://","").replace("www.","").split('/', 1)[0] in new_url:
            urlsToCrawl_Parent.append(new_url)

        elif new_url.startswith('/'):
            new_url = re.sub('\/*$', '', new_url)
            url = re.sub('\/*$', '', url)
            urlsToCrawl_Parent.append(url+new_url)

        else:
            urlsToCrawl_Child.append(new_url)

    return ( urlsToCrawl_Parent, urlsToCrawl_Child )

For det første gjør det algoritmen lettere å lese. For det andre har du separert ut ansvar etc. Det er også lett å se at nå som du short-circuiter hele greien kan du også bryte if-biten opp i flere individuelle sjekker i stedet for en massiv og grusom and.

 

Det er plenty av andre områder du kan fikse på og, men dette er det mest åpenbare.

[PhelpsTransposed]

Tusen takk for tilbakemelding! Bør jeg bryte opp den if'en til flere if'er med continue på hver?

[Lycantrophe]

Det å and-e conditions er greit, men det er ofte greit å bryte i flere mindre når action uansett er å returne eller skippe.

 

edit: sammenlign

 

if any(new_url in word for word in urls_Crawled) and any(new_url in word for word in urlsToCrawl_Parent)and any(new_url in word for word in urlsToCrawl_Child) and '<' in new_url and '#' in new_url and any(word in new_url.split('.')[-1] for word in allowedList) and 'https://wow' in new_url and 'mailto'in new_url:
continue

og

 

if any(new_url in word for word in urls_Crawled):
    continue

if any(new_url in word for word in urlsToCrawl_Parent):
    continue

if any(new_url in word for word in urlsToCrawl_Child):
    continue

if '<' in new_url and '#' in new_url:
    continue

if any(word in new_url.split('.')[-1] for word in allowedList):
    continue

if 'https://wow' in new_url and 'mailto'in new_url:
    continue

Hva er lettest å følge?

 

--

 

En ting til - du gjør oppslaget ditt i en ren array. Det er uhyre mye mindre effektivt enn det kunne ha vært.

Endret 11. mai 2014 av Lycantrophe

[PhelpsTransposed]

Takk igjen! Ser at jeg faktisk sjekket for http://wow ikke http://www. Rart de ikke spurte om jeg var full når jeg skrev koden.

Tenker du på den første for new url in findall, eller parenturls og childurls arrayene?

[Lycantrophe]

child/parent urls.

[PhelpsTransposed]

Har du et forslag/hint til hvordan jeg heller burde gjøre det?

En del av oppgaven var at programmet måtte kunne fortsette crawlingen om det ble abortet, derfor lagret jeg alt til fil for hver gang.

[Matsemann]

Jepp, kjenner mange som har løst samme oppgave. Sitter på tlf., så python med whitespace blir smått uleselig. Det jeg legger merke til er kommentarene. Mange av dem er greie, og forklarer hvorfor du gjør ting. F.eks "this is to be sure that the new url is not in the urls already crawled, or scheduled for crawling".

Men en del kommentarer sier *hva* koden gjør, f. eks "this is if we deal with a local link". Her kan du heller lage en funksjon som heter isLocalLink, og så er koden selvforklarende. Da slipper du også "If we are not dealing with an local URL we are dealing with a child URL", siden koden din basically blir:

If (isLocalUrl(url)){noe} else {noe annet}

 

Edit: Så ikke det var en side til

Endret 11. mai 2014 av Matsemann

[Lycantrophe]

Du kan gjøre child/parent urls til sets (i praksis, hash tables) siden du uansett bare sjekker eksistens. Det er ikke gitt at det er raskere - det kommer an på input og hash-funksjonen som er brukt. Men om du skal gjennom 100 000 urls er det god sjanse for at det lønner seg med O(1) eller O(logn) lookup.

[PhelpsTransposed]

Kult! Løste den nok bedre enn meg siden jeg ikke fikk intervjuet, hehe.

Godt poeng, tusen takk for tilbakemelding på det. Skal bli morro å pusse på koden så den faktisk er brukende.

Du kan gjøre child/parent urls til sets (i praksis, hash tables) siden du uansett bare sjekker eksistens. Det er ikke gitt at det er raskere - det kommer an på input og hash-funksjonen som er brukt. Men om du skal gjennom 100 000 urls er det god sjanse for at det lønner seg med O(1) eller O(logn) lookup.

Aha

Endret 11. mai 2014 av PhelpsTransposed

[snippsat]

Eller nei. Det å matche noen deler av et html-doc med regex er helt greit.

 

Det kan være greit i noen tilfeller,ikke uenig i det.

 

Skal se mer over kode senere @PhelpsTransposed.

Kan starte med:

# The parameter is the url the crawler is gonna crawl.
def crawler(url):
    # The crawler find links by utilizing pythons urllib and the href tag
    for new_url in re.findall('''href=["'](.[^"']+)["']''', urllib.urlopen(url).read()):
        new_url = re.sub('\/*$', '', new_url)

Når du skal dokumentere en funksjon gjøre det med doc-string format.

def crawler(url):
    '''This function is doing this'''

Regex du bruker for finne alle href tag,kan være med XPath være ('//*/@href').

For og ta det i lxml blir det sånn.

from lxml.html import parse

html = parse('url')
href_link = html.xpath('//*/@href')

Try/Catchen kastet jeg på etter at jeg kjørte en live test på 100 000 url'er, og så at det var mye rare enkelt-ting som ødelagte crawlinga.

 

Se litt på Scrapy,der det og skrape 100 000 sider eller mer er vanlig.

Endret 11. mai 2014 av snippsat

[torbjørn marø]

Jeg ser du har fått en bra gjennomgang på codereview fra en Winston Ewert allerede. Det aller siste han sier mener jeg er det viktigste av alt: Lag en topp-level funskjon som tydelig forteller hva algoritmen er, og plasser detaljene i funksjoner som du delegerer til. Skriv kode for mennesker, ikke for maskiner!

Endret 12. mai 2014 av torbjørn marø

[PhelpsTransposed]

Tilbakemeldingene har vært mer enn hva jeg hadde forventet - utrolig kjekt! Takk for all hjelp her også.

 

Your code looks like you tried to solve the problem, and added a series of hacks on top of it to make it work without consideration towards whether or not that was really the correct solution. You just kept piling stuff onto the code and now it's a mess.

Dette tror jeg stemmer bra.

[torbjørn marø]

Tilbakemeldingene har vært mer enn hva jeg hadde forventet - utrolig kjekt! Takk for all hjelp her også.

 

Du har fått gode muligheter til å lære nå, så pass på at du utnytter det og forsøker å kode noe med det du har lært mens det er ferskt. Jeg ville ha begynt med å implementere en ny webcrawler fra scratch...

[PhelpsTransposed]

Helt klart!

Tenkte å ta utgangspunkt i den siste kodesnutten (som du kommenterte på) og jobbe utifra den.

[Lycantrophe]

Hva er bakgrunnen din, om jeg tør spørre?

[PhelpsTransposed]

Vet ikke om jeg tør svare.. NTNU student innenfor en av datalinjene som ikke har progget noe spesielt i python før. Som et lite forsvar var dette venstrehåndsarbeid hvor jeg startet med noe som fungerte OK, som jeg deretter la på flere og flere funksjoner på for å fikse feil URL'er.. Ønsket bare å få noe fungerende.

[Lycantrophe]

For eventuell kritikk i fremtiden - hvor streng (/krass) jeg er avhengig av hva bakgrunnen er. Dersom man er 3.-året datateknikk er standarden langt høyere enn om du er mid-VGS og egentlig ikke har særlig erfaring.

[N*]

 

Hvordan går prosjektet ditt?

 

Har nå fått investorer som spør om investeringsløsninger. Jeg ser på det, og det faktum at jeg nylig har blitt lagt til som Stortingskontakt, som at det er grønt lys for å gi hele Velopt til UtenriksDepartementet. For å reise u-land med total Veloptiv pengeinflasjon. Velopt er 10Trillioner pr.mnd på sitt høydepunkt. Det er de utregningene jeg har gjort på det området.

 

 

 

Altså, Dundas og dere andre som kommer og fornærmer og terger i tråden min... Dere er søte.