3
\$\begingroup\$

I created a PyCryptoScraper script to scrape crypto-currencies from Coingecko. Now I want to know if you could give me some hints or improvements for my script, which works fine.

The whole code also can be downloaded on GitHub.

Dependencies

marshmallow

marshmallow is an ORM/ODM/framework-agnostic library for converting complex datatypes, such as objects, to and from native Python datatypes.

marshmallow can be installed with pip:

pip install -U marshmallow --pre

Usage

To run PyCryptScraper you need to instantiate the class CryptoScraper:

cryptoscraper = CryptoScraper(currencies_to_scrape=[dict(name='bitcoin', tags=['usd', 'eur'])])

or you can execute the python script Scraper.py with certain parameters:

python Scrape.py -c "[{'name':'bitcoin','tags':['usd','eur']}]"

My code looks like so:

Scraper.py

# coding: utf-8
from __future__ import absolute_import, unicode_literals

from scraper.CryptoScraper import CryptoScraper
from schema.DataSchema import DataSchema
import optparse
import sys
import ast
import json

def scrape():
    crypto_scraper = CryptoScraper(currencies_to_scrape=valid_currencies)

    results = crypto_scraper.scrape()

    print json.dumps(results)

if __name__ == "__main__":
    parser = optparse.OptionParser("usage: %prog [options] arg1")
    parser.add_option("-c", "--currencies", dest="currencies", type = "str", help = "specify currencies to scrape")
    parser.add_option("-o", "--output", dest="output", type="str", default="json", help="specify output format")

    (options, args) = parser.parse_args()

    if len(args) != 0:
        parser.error("incorrect number of arguments")

    currencies_to_scrape = ast.literal_eval(options.currencies)
    output = options.output

    # validate currency data
    valid_currencies, errors = DataSchema(many=True).load(currencies_to_scrape)

    if errors:
        print 'error validating data'
        print "data-format:  [{'name':'bitcoin','tags':['usd','eur']}]"
        sys.exit(0)

    # scraping the currencies
    scrape()

scraper/Base.py

# coding: utf-8

from __future__ import absolute_import, unicode_literals

from abc import ABCMeta, abstractmethod


class CryptScraperBase(object):
    """
        Base class for the crypto-scraper
    """
    __metaclass__ = ABCMeta

    @abstractmethod
    def _extract(self, *args, **kwargs):
        """ extracting data"""

    @abstractmethod
    def _transform(self, *args, **kwargs):
        """ transform data"""

scraper/CryptoScraper.py

# coding: utf-8

from __future__ import absolute_import, unicode_literals
from scraper.Base import CryptScraperBase
from schema.DataSchema import DataSchema

from lxml import html
import re
import time
import requests


class CryptoScraper(CryptScraperBase):

    def __init__(self, currencies_to_scrape):
        """
            Initializes the crypto-scraper
        :param currencies_to_scrape: Currencies to scrape
                                    format:  [{ name => "bitcoin", tags => ["usd", "eur"] }, ...]
        :type currencies_to_scrape: list
        """
        self.base_url = 'https://www.coingecko.com/de/kurs_chart/'

        valid_currencies, errors = DataSchema(many=True).load(currencies_to_scrape)
        if not errors:
            self.currencies_to_scrape = valid_currencies
        else:
            # TODO: Throw in error if the currencies are not valid
            pass

    def _extract(self, name, tag, *args, **kwargs):
        """
            Extract data from the given url
        :param name: CryptCurrency name
        :type name: str
        :param tag: Tag for the CryptCurrency
        :type tag: str
        :param args:
        :param kwargs:
        :return:
        """
        status_code, content = self.get_page_content(name, tag)
        if status_code == 200:
            return content
        else:
            raise ('page is not available')

    def _transform(self, page_content, exchange_rate, *args, **kwargs):
        """
            Transforms the extraced data into an given format
        :param page_content: extraced page content in html
        :type page_content: str
        :param args:
        :param kwargs:
        :return: Transformed CryptCurrency Details
        """
        return self._get_currency_details(page_content, exchange_rate)

    def scrape(self):
        """
            Scrape Crypt-Currencies from the given page
        :return: Scraped Crypt-Currencies
        """
        results = []

        for currency in self.currencies_to_scrape:
            name = currency.get('name')
            tags = currency.get('tags')

            for tag in tags:
                page_content = self._extract(name=name, tag=tag)
                crypt_currency = self._transform(page_content=page_content, exchange_rate=tag)
                results.append(crypt_currency)

        return results

    def get_page_content(self, name, tag):
        """
            Returns content from the page crypto-currencies are listed
        :param name: Crypto-Currency name
        :type name: str
        :param tag: The Tag for the crypto-currency (usd, eur, ..)
        :type tag: str
        :return: The page content and statuscode
        """
        response = requests.get(self.base_url+name+'/'+tag)
        return response.status_code, response.content

    def clean_currency_amount(self, data):
        m = re.search(r'^\s+(.*\d+)\s+.*', data)
        if m:
            return m.group(1)

    def _get_currency_details(self, page_content, exchange_rate):
        """
            Get all details for an crypt-currency
        :param page_content: The html page content with crypt-currency information
        :type page_content: str
        :return: extracted details for an given crypt-currency
        """
        tree = html.fromstring(page_content)

        # Options for xpath
        opts = {
            'currency': '1',
            'code': '2',
            'exchange_rate': '3',
            'market_capitalisation': '4',
            'trading_volume': '5'
        }

        crypt_currency = dict()

        for key, value in opts.iteritems():
            info = tree.xpath('//div[@class="col-xs-12"]/div/table[@class="table"]/tbody/tr/td[' + value + ']/text()')[0]
            if value in('3', '4', '5'):
                crypt_currency[key] = self.clean_currency_amount(info)
            else:
                crypt_currency[key] = info

        # append actual timestamp
        crypt_currency['date'] = int(time.time())

        # append the exchange_rate
        crypt_currency['exchange_rate'] = exchange_rate.upper()

        return crypt_currency

schema/DataSchema.py

# coding: utf-8

from __future__ import absolute_import, unicode_literals

from marshmallow import Schema, fields


class DataSchema(Schema):
    name = fields.String(required=True)
    tags = fields.List(fields.String(), required=True)
\$\endgroup\$
2
\$\begingroup\$

Awesome job overall, it is a breeze to review a structured code like this.

Here are some of notes I've come up with:

  • optparse was deprecated long time ago, you should switch to argparse
  • clean_currency_amount() is actually a static method and should not be under the CryptoScraper class - probably it should be under the "utils" module
  • I don't particularly like the way you've implemented _get_currency_details() - the way you define if a cell is of an "amount" type - I would do something more explicit, e.g.:

    headers = [
        ('currency', 'string'),
        ('code', 'string'),
        ('exchange_rate', 'amount'),
        ('market_capitalisation', 'amount'),
        ('trading_volume', 'amount')
    ]
    
    table = tree.xpath("//th[. = 'Kurs']/ancestor::table")[0]
    values = [cell.text for cell in table.findall(".//td")]
    
    crypt_currency = {}
    for (header, header_type), value in zip(headers, values):
        crypt_currency[header] = clean_currency_amount(value) if header_type == 'amount' else value
    

    Note, also, how I locate the desired table - I'm basically using a header name to find the "exchange" table - it is a bit better than what you current have because you are using the layout-oriented bootstrap class and have multiple parent nodes checked on the way to the desired element making the expression more fragile and less resistant to a change.

\$\endgroup\$
  • \$\begingroup\$ Great. Thanks a lot for your review an helpful hints. I will apply your suggestions on my code. THANKS :) \$\endgroup\$ – Patrick85 Jul 2 '17 at 9:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.