4
\$\begingroup\$

I wrote a program to download a manga series fron www.mangapanda.com

Here it is:

import os

import requests
from lxml import html

from cleanname import clean_filename

dir_loc = r''
website_url = r'https://www.mangapanda.com'
manga_url = r'https://www.mangapanda.com/one-piece'


def check_url(url):
    url_status = requests.head(url)
    if url_status.status_code < 400:
        return True
    return False


def scrap_chapter_list(url, respose):
    dic = {'chapter': '', 'name': '', 'link': ''}

    # start scrapping
    # soup  = BeautifulSoup(respose.text,'html.parser')
    tree = html.fromstring(respose.content)
    return None


def get_list_of_chapers(url):
    if check_url(url):
        response = requests.get(url).content
        tree = html.fromstring(response)
        path = r'//*/div[@id="chapterlist"]/table[@id="listing"]/tr/td/a'
        res = tree.xpath(path)
        dic = {'chapter': '', 'url': '', 'name': ''}
        result = []
        for i in res:
            dic['chapter'] = i.text
            dic['url'] = website_url + i.attrib['href']
            dic['name'] = i.tail
            result.append(dic)
            dic = {'chapter': '', 'url': '', 'name': ''}
        return result
    return None


def get_page_list(chapter_url):
    res = requests.get(chapter_url).content
    path = r'//*/div[@id="selectpage"]/select[@id="pageMenu"]'
    tree = html.fromstring(res)
    data = tree.xpath(path)[0]
    page_links = ['{}'.format(i.attrib['value']) for i in data]
    return page_links


def get_image_from_page(url):
    """

    :param url:  url of the given manga page eg. /one-piece/1/1
    :return: name of the page(manga name, link to the image file
    """
    dic = {'page_name': '', 'source': ''}
    page_url = r'{}{}'.format(website_url, url)
    res = requests.get(page_url).content
    path = r'//*/img[@id="img"]'
    tree = html.fromstring(res)
    result = tree.xpath(path)
    dic['page_name'], dic['source'] = result[0].attrib['alt'], result[0].attrib['src']
    return dic


def download_image(image_url):
    image_file = requests.get(image_url).content
    return image_file


def save_file(image_file, location, filename, img_format):
    image_loc = os.path.join(location, filename)+img_format
    with open(image_loc, 'wb') as file:
        file.write(image_file)
    return True if os.path.isfile(image_loc) else False


def get_page_details(chapter_url):
    dic = {'page_link': '', 'page_name': '', 'source': ''}
    page_details = get_page_list(chapter_url)
    result = []
    for page in page_details:
        details = get_image_from_page(page)
        dic['page_link'] = page
        dic['page_name'], dic['source'] = details['page_name'], details['source']
        result.append(dic)
        dic = {'page_link': '', 'page_name': '', 'source': ''}
    return result


# if __name__ == '__main__':
#     from .cleanname import clean_filename
manga_url = r'https://www.mangapanda.com/akame-ga-kiru'
storing_location = r'C:\Users\prashra\Pictures\mangascrapper'
manga_name = manga_url.split('/')[-1]
location = os.path.join(storing_location, clean_filename(manga_name))
chapter_list = get_list_of_chapers(manga_url)[:6]

if not os.path.exists(location):
    print('creating the folder {}'.format(manga_name))
    os.makedirs(location)

for chapter in chapter_list:
    name = r'{} {}'.format(chapter['chapter'], chapter['name'])
    chapter_path = os.path.join(location, clean_filename(name))
    print(chapter_path)
    if not os.path.exists(chapter_path):
        os.makedirs(chapter_path)
    chapter_details = get_page_details(chapter['url'])
    for _page in chapter_details:
        name, src = _page['page_name'], _page['source']
        img_format = '.' + src.split('.')[-1]
        print('saving image {} in path {}'.format(name, chapter_path))
        image_data = requests.get(src).content
        save_file(image_data, chapter_path, name, img_format)

and in cleanname.py file

import unicodedata
import string

valid_filename_chars = "-_ %s%s" % (string.ascii_letters, string.digits)
char_limit = 255


def clean_filename(filename, whitelist=valid_filename_chars, replace='_'):
    # replace spaces
    for r in replace:
        filename = filename.replace(r, '_')

    # keep only valid ascii chars
    cleaned_filename = unicodedata.normalize('NFKD', filename).encode('ASCII', 'ignore').decode()

    # keep only whitelisted chars
    cleaned_filename = ''.join(c for c in cleaned_filename if c in whitelist)
    if len(cleaned_filename) > char_limit:
        print(
            "Warning, filename truncated because it was over {}. Filenames may no longer be unique".format(char_limit))
    return cleaned_filename[:char_limit]

I want to ask :

  1. review on this code
  2. is it better to convert the code in classes form
  3. how to make it scaleable, like to download a chapter only not entire list
\$\endgroup\$
4
\$\begingroup\$

Just going to focus on your main program, not cleanname.py.

  • Import Order: This one is a personal preference. I like to have my imports ordered alphabetically. You can decide to follow this practice or not.
  • Docstrings: Docstrings are used to help identify what your method is supposed to do. You should include one in every method you write.
  • Unused variables/methods: You had a few unused variables and one method. You should remove these to improve the readability of your program, so someone doesn't have to look for 10 minutes (those without a linter, of course) before realizing it's never used in your code.
  • Return expressions: if url_status.status_code < 400: return True ... return False can be reduced to simply return url_status.status_code < 400. Returning expressions is much better than returning True/False, improves readability greatly.
  • Useless returns: If you're not expecting to return anything in a method, leave it as it is. Having the line return None is redundant, because any method that doesn't return anything automatically returns None.
  • Using f"" over "".format(...): This one is another preference, but having my strings formatted using f"..." looks much cleaner than using .format. It allows you to directly implement variables into your strings, rather than concatenating or using the format method.
  • Return/append anonymously: In many places, you create a dict, change it, append to a list, then reset the dict. That's a lot of steps for adding a dict to a list. You can simply appending an anonymous dict result.append({ ... }) or return { ... }, so you don't need to keep creating/resetting a dict you're only using in that method.
  • Main guard: You should wrap any code that isn't in a function in a main guard. Here is an answer that provides a deeper and more meaningful explanation.
  • Constant Variable Names: Variables that are constants should be UPPERCASE.

Final Code

import os
import requests
from cleanname import clean_filename
from lxml import html

def check_url(url):
    """ Returns the status code being less than 400 """
    return requests.head(url).status_code < 400

def get_list_of_chapters(url):
    """ Returns a list of chapters from the specified `url` """
    if check_url(url):
        response = requests.get(url).content
        tree = html.fromstring(response)
        path = r'//*/div[@id="chapterlist"]/table[@id="listing"]/tr/td/a'
        res = tree.xpath(path)
        result = []
        for i in res:
            result.append({
                'chapter': i.text,
                'url': WEBSITE_URL + i.attrib['href'],
                'name': i.tail
            })
        return result
    return None


def get_page_list(chapter_url):
    """ Returns a list of link to the page on `chapter_url` """
    res = requests.get(chapter_url).content
    path = r'//*/div[@id="selectpage"]/select[@id="pageMenu"]'
    tree = html.fromstring(res)
    data = tree.xpath(path)[0]
    page_links = [f"{i.attrib['value']}" for i in data]
    return page_links

def get_image_from_page(url):
    """ Gets the image from `url` """
    page_url = f"{WEBSITE_URL}{url}"
    res = requests.get(page_url).content
    path = r'//*/img[@id="img"]'
    tree = html.fromstring(res)
    result = tree.xpath(path)
    return {
        'page_name': result[0].attrib['alt'],
        'source': result[0].attrib['src']
    }


def download_image(image_url):
    """ Downloads image from `image_url` """
    return requests.get(image_url).content


def save_file(image_file, save_location, filename, save_img_format):
    """ Saves the `image_file` to `location` with the name `filename` """
    image_loc = os.path.join(save_location, filename) + save_img_format
    with open(image_loc, 'wb') as file:
        file.write(image_file)
    return os.path.isfile(image_loc)


def get_page_details(chapter_url):
    """ Gets the details about the page """
    page_details = get_page_list(chapter_url)
    result = []
    for page in page_details:
        details = get_image_from_page(page)
        result.append({
            'page_link': page,
            'page_name': details['page_name'],
            'source': details['source']
        })
    return result


if __name__ == '__main__':

    DIR_LOC = r''
    WEBSITE_URL = r'https://www.mangapanda.com'
    MANGA_URL = r'https://www.mangapanda.com/one-piece'

    MANGA_URL = r'https://www.mangapanda.com/akame-ga-kiru'
    STORING_LOCATION = r'C:\Users\prashra\Pictures\mangascrapper'
    MANGA_NAME = MANGA_URL.split('/')[-1]
    LOCATION = os.path.join(STORING_LOCATION, clean_filename(MANGA_NAME))
    CHAPTER_LIST = get_list_of_chapters(MANGA_URL)[:6]

    if not os.path.exists(LOCATION):
        print(f"Creating folder: {MANGA_NAME}")
        os.makedirs(LOCATION)

    for chapter in CHAPTER_LIST:
        name = rf"{chapter['chapter']}{chapter['name']}"
        chapter_path = os.path.join(LOCATION, clean_filename(name))
        print(chapter_path)
        if not os.path.exists(chapter_path):
            os.makedirs(chapter_path)
        chapter_details = get_page_details(chapter['url'])
        for _page in chapter_details:
            name, src = _page['page_name'], _page['source']
            img_format = f".{src.split('.')[-1]}"
            print(f"Saving image {name} in path {chapter_path}")
            image_data = requests.get(src).content
            save_file(image_data, chapter_path, name, img_format)
\$\endgroup\$
  • 1
    \$\begingroup\$ thanks for the valubable feedback, I like the way you explain and all the points you mentioned, will consider all points and implement it in the code, just disagreeing of using "f" over ''.format() part, i find it more readable and more customisable , will stick to this part , rest i like they way you did inline checking of url status and overocome the headache of creating new dict everytime \$\endgroup\$ – sahasrara62 Jul 25 '19 at 7:33

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.