2
\$\begingroup\$

NOTE

The script relies on ruamel-yaml and requests packages available on pypi.

I wrote a script which goes over the pack file details as specified by the emojipacks project, and downloads (and saves) to the local drive. It downloads the files as threads, and saves to disk with the shutil's copyfileobj method.

Feel free to nitpick each and everything which could be improved.

from argparse import ArgumentParser
from pathlib import Path
from shutil import copyfileobj
from threading import Thread

import requests
import ruamel.yaml as YAML

yaml = YAML.YAML(typ="safe")
DOWNLOAD_DIR: str = "emojis"


def download_file(url: str, filename: str, parent: Path):
    """
    Downloads the file from `url` and saves to `filename` inside the `parent` directory.
    """
    suffix = url.split(".")[-1]
    if suffix in ("png", "jpg", "gif", "jpeg"):
        filename = f"{filename}.{suffix}"
    path = Path(parent / filename)
    with requests.get(url, stream=True) as response:
        with path.open(mode="wb") as f:
            copyfileobj(response.raw, f)


def create_download_thread(url: str, name: str, path: Path) -> Thread:
    thread = Thread(target=download_file, args=(url, name, path))
    thread.start()
    return thread


def main(dir_path: str, download_dir: str):
    path = Path(dir_path)
    threads = []
    if not path.is_dir():
        return
    for yaml_file in path.glob("*.yaml"):
        data = yaml.load(yaml_file)
        emoji_dir = Path(download_dir) / data["title"]
        emoji_dir.mkdir(parents=True, exist_ok=True)
        for emoji in data["emojis"]:
            names = [emoji.get("name"), *emoji.get("aliases", [])]
            src = emoji.get("src")
            threads += [create_download_thread(src, name, emoji_dir) for name in names]
    for thread in threads:
        thread.join()


if __name__ == "__main__":
    parser = ArgumentParser(description="Download emojipacks to specified directory!")
    parser.add_argument(
        "directory",
        nargs="+",
        help="Directory(ies) with yaml for the emojipack specifications.",
    )
    parser.add_argument(
        "-p",
        "--download-path",
        help="Directory where emojis will be downloaded.",
        default=DOWNLOAD_DIR,
    )
    args = parser.parse_args()
    for path in args.directory:
        main(path, args.download_path)

The script was created because the project emojipacks itself is no longer maintained and the authentication system has been updated from slack. The downloaded emojis are then bulk uploaded to slack using another utility slack-emojinator.


EDIT

Since this point was raised in the answer(s), the yaml specification is:

Also note that the yaml file must be indented properly and formatted as such:

title: food emojis:
  - name: apple
    src: http://i.imgur.com/Rw0Vlda.png
  - name: applepie
    src: http://i.imgur.com/g4RU1fM.png

..with the src pointing to an image file. According to Slack:

[...]

It is possible to give multiple names to a single emoji using yaml such as:

title: octicons emojis:
  - name: pr
    aliases:
      - pullrequest
      - mergerequest
    src: https://i.imgur.com/rhwNxfc.png

emphasis mine

\$\endgroup\$
2
\$\begingroup\$

Parametrization

DOWNLOAD_DIR is being used as a default for a command-line argument. This is surprising, since at first glance one would assume that this global constant is simply being used as the download directory value itself. It's less clutter and more obvious if you simply remove this global and write the string literal beside the default kwarg.

More pathlib

You're already using it, but you can benefit from it more. Rather than making a Path() here:

path = Path(parent / filename)
  1. Use urllib.parse.urlparse to get you a ParseResult with a path component, since it's a more thorough and safe parsing method than bare split and you only care about path

  2. Call path.rsplit('/', 1)[-1] - not split - on the path component to get the filename-like

  3. Make a Path from the result

  4. If you deeply care about image-likes, then check mypath.suffix for .png, etc.

  5. Use / as you are now.

Set membership

if suffix in ("png", "jpg", "gif", "jpeg"):

should use a set for speed, and also because it more closely matches your meaning. Also, pathlib will need leading dots:

if suffix in {'.png', '.jpg', '.gif', '.jpeg'}:

Noisy failure

if not path.is_dir():
    return

does not help the user understand why the script quit with no message. Raise some kind of error here.

Fail-safe list construction

The first part of this expression:

[emoji.get("name"), *emoji.get("aliases", [])]

will not do what you want. If there is no 'name', a None will be inserted rather than nothing. A more "naive" approach that will actually work is:

names = emoji.get('aliases', [])
name = emoji.get('name')
if name is not None:
    names.append(name)

Order probably doesn't matter to you since you had been kicking off threads all at once.

Somewhat-related: is this -

src = emoji.get("src")

actually optional? I doubt it. Currently you require that it be an actual URL. Either use [] if you want this to keep being fatal, or:

src = emoji.get("src")
if src is not None:
    create_download_thread ...

List append of a comprehension

        threads += [create_download_thread(src, name, emoji_dir) for name in names]

creates a temporary list only to throw it away again once it's been appended. The following will use the generator but not materialize it to a temporary list:

threads.extend(create_download_thread(src, name, emoji_dir) for name in names)
\$\endgroup\$
4
  • \$\begingroup\$ 1. I do have download-path cli param. 2. I am extracting suffix from the download url, because almost all the links do not return Content-Disposition header in response. 3. I was thinking of validating against name and src data, but the emojipack yaml requires those fields to be present, the only optional candidate is aliases. 4. .extend and noisy failures do make sense. I'll add those. \$\endgroup\$ – hjpotter92 Nov 23 '20 at 15:09
  • \$\begingroup\$ Re. 1, I still think some improvement can be had, but I've edited my answer for nuance. \$\endgroup\$ – Reinderien Nov 23 '20 at 15:14
  • 1
    \$\begingroup\$ Re. 2: I know that you're extracting the suffix from the URL; my proposal is that you continue to do that but using slightly more robust parsing. \$\endgroup\$ – Reinderien Nov 23 '20 at 15:15
  • 1
    \$\begingroup\$ Re. 3: if those fields are required to be present, then do not get at all; use direct indexing [ ] - because the lack of those fields should be more of a surprise; fail as early as possible rather than filling your data with Nones. \$\endgroup\$ – Reinderien Nov 23 '20 at 15:17

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.