Data extraction

Question

I wrote this code in Python/ Scrapy for data extraction. It works fine for me, but I am a newbie in Python, so any suggestions for code optimization would be welcome.

def parse(self, response):
    hxs = HtmlXPathSelector(response)
    item = GenesishcsItem()
    item['graduate_education']=[]
    item['medical_school']=[]
    item['specialty']=[]
    item['name']=ParseUtils.get_line_from_node(hxs.select(".//*[@id='DrDetail']//div[@class='Designations']/h1"))
    image_url=hxs.select("//img[contains(@id,'DoctorProfilePicture')]/@src").extract()
    if image_url:
        item['image_url']=urljoin(response.request.meta['cached_meta']['response_url'],image_url[0])
        for  key,value in {'Gender:':'gender','Year joined staff:':'year_joined_staff'}.iteritems():
            result=ParseUtils.get_line_from_node(hxs.select("//dt[contains(.,'%s')]/following-sibling::dd"%key))
            if result:
                item[value]=result

    edu_data=['Education','Residency','Fellowship','Internships']
    for edu in edu_data:
        result_date=hxs.select("//h4[contains(.,'%s')]/following-sibling::dl[@class='%s']/dt"%(edu,edu))
        result=hxs.select("//h4[contains(.,'%s')]/following-sibling::dl[@class='%s']/dd"%(edu,edu))
        for date,value in zip(result_date,result):
            date=date.select('./text()').extract()
            value=value.select('./text()').extract()
            if date:
                date=date[0]
            else:
                date=None
            if value:
                value=value[0]
            else:
                value=None
            if 'Education' in edu:
                pass
                item['medical_school'].append({'name':value,'graduation_year':date})
            else:
                item['graduate_education'].append({'name':value,'start_year_end_year':date,'type':edu.lower().strip()})
    for rs in ParseUtils.get_text_from_node(hxs.select("//h3[contains(.,'Board Certified')]/following-sibling::p")):
        item['specialty'].append({'name':rs,'certified':True})
    return item

Answer 1

Your code is messy and difficult to understand. I believe that in a few month you will waste time to understand what you have written earlier. So it is can be ok if you are not going to support/extend/re-use your project.

Scrapy offer quite good architecture to organize your code properly: Item Loaders and Input and Output processors, Item Pipeline. It will move processing logic and make your main parse method cleaner.

Answer 2

The code looks good, but as always, there are always issues one can bring up. I don't believe them to be important though. Your code seems to be getting the job code in a concise way, and if your XPath expressions are robust, there's no need to worry.

• Do you have actual performance issues? If so, you should profile your code to see where your program is spending most of its time.
• The pass statement at the end of your code is useless.
• It's a bit sad to see the code duplication when you're doing date=date[0] but then it's a bit hard to avoid in Python. There's always the possibility to write date = date[0] if date else none but you could say it's more difficult to read.
• The way you're reassigning your variables is a bit dangerous since the names refer to different types depending only on the "current line number". For small scripts it's OK.
• Try to find more descriptive names, result and value are vague.
• You could want to "cache" some of the nodes (eg. "//h4[contains(.,'%s')]/following-sibling::dl[@class='%s']) so that there's less duplication in your XPath expressions.