Hot answers tagged

11

It seems that every one of your getCity(), getDepartment() etc functions loads the same web page over and over. You should load each URL once with your curl_get_contents(), then pass its result into each get*() function to parse it.


10

I'll start off by saying this is actually really good. There are a few minor PEP8 errors: In after_login you call Request. the argument url shouldn't have a space on the right side of the equals. Another thing in Request. The argument callback needs at least one more indent. However you should either aligned with opening delimiter or use hanging indents. ...


7

You would do well to use generator functions to break this down into parts that might be re-usable for other steps in your pipeline. For example, the first thing you do in your loop is read a row from the CSV file and process some of the fields. That's a good candidate, right there: def normalized_csv_rows(reader): """Filter CSV data, cleaning up email ...


5

The spider is readable and understandable. I would only extract some of the things into separate methods for readability. For example, the "infinite scroll" should probably be just defined in a separate method. And, the bare except can be replaced with handling a more specific TimeoutException: def scroll_until_loaded(self): check_height = self.driver....


5

There are no docstrings, so a user of your class is left wondering which methods could be useful to call. In fact, after studying the code, it looks like all the methods are meant to be called by __init__, in a specific order. Makes me wonder if this should be a class at all. You set very many instance attributes, even a self.dummy, but use very few outside ...


4

Here are some of the things I would improve in the code: put each import on a separate line, there is not much point in saving space in this case: from lxml import html import requests as usual and I think we've discussed it on CR already - re-use requests.Session() instance to make your requests - this will help to improve on the speed of downloading the ...


4

Even though you are trying to mimic what Scrapy spider might look like, there is a very major high-level difference between how your code is executed and how a Scrapy spider is. Scrapy is entirely asynchronous since it is based on a twisted network library which makes the code operate in a non-blocking nature, to quote the documentation: Requests are ...


4

First: a general observation - simple nested if statements are equivalent to a single if with statements joined by and. So: if a: if b: if c: Is equivalent to: if a and b and c: Second: You have an if test to see if business_name and website exist - but you do a lot of other things before this test. You can move this higher up so you 'escape'...


3

I'd like to mention, that there is a special way of making output files in scrapy - item pipelines. So, in order to make it right, you should write your own pipeline (or modify standard one via subclassing). Also, you does not close the file, once you're done and you keep it open most of the time. The both problems are handled nicely with pipelines. UPD: ...


3

DRY Reduce duplicated logic using helper functions. Currently you have 2 lines of code for each field you extract, for example: companys = sel.xpath('//*[@id="company-name"]/text()').extract() companys = [company.strip() for company in companys] names = sel.xpath('//*[@id="%s"]/a/text()' % string1).extract() names = [name.strip() for name in names] This ...


3

Auto-throttling extension may cause high download delays. Either turn the extension off to see what would the resulting time be, or limit the maximum delay via AUTOTHROTTLE_MAX_DELAY. Also, you may issue multiple requests from the after_login() method instead of keeping the queue of references: def after_login(self, response): for ref in references: ...


3

Your utilities module is too specific Separating the code into data model, utilities functions and proper logic is generally a good idea. But in your code, the utilities module contains functions that are tightly coupled with your spider. I’d rather use them as @staticmethods of your spider. Except maybe for get_exchange_rates and get_base. However, for ...


3

Well to start with you have bad practices in your imports. It's recommended to stay away from using from module import * because doing that imports things without explicitly declaring their names. Without realising it, you could be overwriting other functions, including builtins in the module was made carelessly. Instead use just import module or from module ...


3

I'm having trouble following this code, so I'm going to focus on maintainability. Here are some things you can consider changing: Some members are currently initialized in __init__, some are only initialized in other methods. It's typically good to define invariants of your class that will remain true at all observable times. Since __init__ calls everything ...


3

It's hard to analyze the bottle neck without performance data on where the bottleneck is occurring. You should consider running the program several ways and comparing. Ie run program as is run program spider only with no db This should tell you the problem is in your code as you suspect. If not you are crawling something that is slow to crawl. Your ...


3

You can use Field() default values as suggested. class MalCrawlerItem(Item): mall = Field(default='null') store = Field(default='null') bonus= Field(default='null') per_action = Field(default='null') more_than = Field(default='null') up_to = Field(default='null') deal_url = Field(default='null') category = Field(default='null'...


2

def sp_offer_page(self, response): item = MallCrawlerItem() If MallCrawlterItem is your class, it might make more sense to have it set everything to defaults. for key, value in self.get_default_item_dict().iteritems(): item[key] = value MallCrawlerItem appears to be dict-like. dict's have a method update, which can ...


2

The code is quite clean and easy to read, good job! I would only focus on couple things: remove extra spaces around = when it is used to define a keyword argument CSS selectors are more appropriate and reliable when it comes to handling multi-valued class attribute than XPath expressions. Plus, they are more concise and generally faster naming - for links ...


2

Great job overall! I like how simple and readable the usage sample classes are. Here are some of my high-level overview thoughts. Code Organization The class feels more like a "God class", since it combines a lot of not directly related things - for instance, it has this _clear_text_field helper, different _find* and _extract* methods and parse* ...


2

Method The fastest way to check if an item into a MongoDB is unique (and if it isn't, not insert it) is to create a unique index on the related columns and catch the error upon insertion time. Watch more here. MongoDB - Features Maybe this would be OT, but I would like to let you know all this: MongoDB is inconsistent by default. The documentation ...


2

The main problem is that you are writing/appending to the file inside the spider's parse() method - this is not how this should be done in Scrapy - there is a special place - Item Pipelines. Check this answer to see how to include a custom pipeline when running Scrapy from a script. Note that in the pipeline you should not be appending to a file, but ...


2

By putting the CSV exporting logic into the spider itself, you are re-inventing the wheel and not using all the advantages of Scrapy and its components and, also, making the crawling slower as you are writing to disk in the crawling stage every time the callback is triggered. As you mentioned, the CSV exporter is built-in, you just need to yield/return ...


1

Don't abuse inner lists This: self.name = ' '.join([data.get('name').get(key) for key in ['first_name', 'last_name']]) should be self.name = ' '.join(data.get('name', {}}.get(key) for key in ('first_name', 'last_name')) Note the following: Generators don't need to go in a list if they're just being passed to a function (join) that needs an iterable ...


1

You should opt for .close() method as I've tried below. This method will be called automatically once your spider is closed. class SuborgSpider(scrapy.Spider): name = "suborg" start_urls = ['https://www.un.org/sc/suborg/en/sanctions/1267/aq_sanctions_list/summaries?type=All&page={}'.format(page) for page in range(0,7)] outfile = open("...


1

You should ensure that the file is closed. In addition you should avoid creating a new writer object every loop iteration using the with statement: class GetInfoSpider(scrapy.Spider): name = "infrarail" start_urls= ['http://www.infrarail.com/2018/exhibitor-profile/?e={}'.format(page) for page in range(65,70)] output = "output.csv" def ...


1

I don't think you should reinvent the wheel and provide your own CSV export. The following works for me as is (note the addition of .strip() calls - though I don't think they are necessary at all): import scrapy class ToscrapeSpider(scrapy.Spider): name = "toscrapesp" start_urls = ["http://books.toscrape.com/"] def parse(self, response): ...


1

A good place to close the driver would be the the closed method, which is called when the spider is closed: def closed(self, reason): self.driver.close()


1

You can create an index on that field: db.collection.createIndex({"field":"",{unique:true}) Mongo will throw an error if the same value is being inserted.


1

For your requests list, I suggest defining a dictionary like that: requests_dict = { 'putnam': 'http://www.putnam-fl.com/clerks_web_apps/foreclosure_trans/get_cases.php', 'palmbeach': 'https://mypalmbeachclerk.clerkauction.com/', ... } and then doing something like: requests_list = [Request(url, callback=getattr(self, 'parse_'+name)) for name, ...


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 ...


Only top voted, non community-wiki answers of a minimum length are eligible