9
\$\begingroup\$

I wrote the following script to parse an SEP article and call pandoc to convert it to EPUB. I'd love your feedback.

There is no function but I didn't think it was worth adding. Also there is no test to see if the file is executed or imported, but since it is supposed to be an executable this is not a problem in my opinion.

"""
sep2epub
"""

import os

from bs4 import BeautifulSoup
import argparse
import requests
import yaml

# Parse command line arguments

parser = argparse.ArgumentParser("Create an epub file from an article in the Stanford Encyclopedia of Philosophy")
parser.add_argument("id", help="Identifier of the article")

args = parser.parse_args()
URL = "https://plato.stanford.edu/entries/" + args.id

# Request the HTML page and parse it with BeautifulSoup

response = requests.get(URL)
soup = BeautifulSoup(response.text, "html.parser")

article_content = soup.find("div", id="article-content")
article_title = article_content.find("h1").text

article_content.find("div", id="academic-tools").decompose()
article_content.find("div", id="related-entries").decompose()

# Export HTML and metadata

os.makedirs("tmp", exist_ok=True)

with open(f"tmp/{args.id}.html", "w") as f:
    f.write(str(article_content))

metadata = {
    "title": article_title,
    "publisher": "Stanford Encyclopedia of Philosophy",
}

with open("tmp/metadata.yaml", "w") as f:
    f.write(yaml.dump(metadata,
        explicit_start=True, explicit_end=True))

# Use pandoc to generate the epub file

command = f"pandoc tmp/{args.id}.html"
command += " --metadata-file=tmp/metadata.yaml"
command += f" -o {args.id}.epub"
os.system(command)
\$\endgroup\$

2 Answers 2

12
\$\begingroup\$

Add a hashbang at the top of your script since it's expected to be executable.

There is no function but I didn't think it was worth adding

Functions are worth adding: the visual and performance overhead is minimal, it increases self-documentation and testability. For these same reasons you should add a __main__ guard.

Since you only care about the DOM subtree under one element, use a strainer.

Don't create your own temporary directory; use the system one via the tempfile library. If you can guarantee that you're not running on Windows, the usage is slightly easier for an auto-deleted file than if you're running on Windows due to file-sharing permissions issues; but in either case tempfile offers better security, potentially better performance (depending on how the OS maps temporary files), better idempotence and generally a file access pattern more in line with what the operating system expects for temporary data.

There's basically never a good case for calling os.system; use subprocess instead. Use check_call so that it stands a chance of detecting when something goes wrong and throwing an exception from the Python stack.

Don't ignore requests network failures; call raise_for_status().

Suggested

#!/usr/bin/env python3

"""
sep2epub
"""
import os
import subprocess
from tempfile import NamedTemporaryFile

from bs4 import BeautifulSoup
from bs4.element import SoupStrainer
import argparse
import requests
import yaml


def parse_args() -> str:
    parser = argparse.ArgumentParser('Create an epub file from an article in the Stanford Encyclopedia of Philosophy')
    parser.add_argument('id', help='Identifier of the article')

    args = parser.parse_args()
    return args.id


def fetch_content(article_id: str) -> tuple[
    str,  # title
    str,  # HTML content
]:
    url = 'https://plato.stanford.edu/entries/' + article_id
    strainer = SoupStrainer(name='div', id='article-content')

    with requests.get(url, headers={'Accept': 'text/html'}) as response:
        response.raise_for_status()
        soup = BeautifulSoup(markup=response.text, features='html.parser', parse_only=strainer)

    content, = soup.contents
    title = content.find('h1').text

    for delete_id in ('academic-tools', 'related-entries'):
        content.find('div', id=delete_id).decompose()

    return title, str(content)


def make_metadata(title: str) -> dict:
    return {
        'title': title,
        'publisher': 'Stanford Encyclopedia of Philosophy',
    }


def export(
    article_id: str,
    content: str,
    metadata: dict,
) -> None:
    try:
        with NamedTemporaryFile(mode='wt', delete=False, suffix='.html', prefix=f'{article_id}-') as html_file, \
             NamedTemporaryFile(mode='wt', delete=False, suffix='.yaml', prefix=f'{article_id}-metadata-') as yaml_file:
            html_file.write(content)
            yaml.dump(data=metadata, stream=yaml_file, explicit_start=True, explicit_end=True)

        # Use pandoc to generate the epub file
        subprocess.check_call(
            (
                '/usr/bin/pandoc',
                html_file.name,
                '--metadata-file', yaml_file.name,
                '-o', f'{article_id}.epub',
            ), shell=False
        )
    finally:
        os.unlink(html_file.name)
        os.unlink(yaml_file.name)


def main() -> None:
    article_id = parse_args()
    title, content = fetch_content(article_id)
    export(article_id, content, metadata=make_metadata(title))


if __name__ == '__main__':
    main()
\$\endgroup\$
1
  • 2
    \$\begingroup\$ Thanks for the corrections, there are some interesting things, I look forward to reading your comments \$\endgroup\$
    – user260644
    Commented Jul 11, 2022 at 12:49
4
\$\begingroup\$

General style

  • Use normal comments instead of triple quoted strings. Actually """ sep2epub """ doesn't really convey anything useful by itself if that is the script name.

  • I would recommend putting everything in functions instead of global scope, but for short scripts it's probably ok.

  • It's recommended to use subprocess over passing a single string to os.system https://stackoverflow.com/questions/4813238/difference-between-subprocess-popen-and-os-system See Reinderien's answer for combining command line arguments

\$\endgroup\$
2
  • 1
    \$\begingroup\$ The first lines of the script are not a comment but a module docstring, hence the triple quotes. But for the last point, it seems interesting, I will do some research, thanks. \$\endgroup\$
    – user260644
    Commented Jul 12, 2022 at 7:59
  • 2
    \$\begingroup\$ ok but the docstring still doesn't convey anything useful which is its job \$\endgroup\$
    – qwr
    Commented Jul 12, 2022 at 16:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.