2
\$\begingroup\$
from subprocess import Popen, PIPE
import requests
import json
import schedule
import time
import datetime
def setwallpaper():
    date = datetime.datetime.now()
    url = 'https://api.nasa.gov/planetary/apod?
api_key= #myApiKey'
    try:
        response = requests.get(url)
        response.raise_for_status()  # Check for any HTTP errors
        print(response.text)
    except requests.exceptions.RequestException as e:
        print(f"Error: {e}")
        print('Error')
    test_string = response.text
    res = json.loads(test_string)
    url = res['url']
    print(url)

    # This statement requests the resource at
    # the given link, extracts its contents
    # and saves it in a variable
    data = requests.get(url).content

    # Opening a new file named img with extension .jpg
    # This file would store the data of the image file
    f = open(f'APOD{date}.jpg', 'wb') #creates new file 

    # Storing the image data inside the data variable to the file
    f.write(data)
    f.close()

    script = f'''tell application "System Events"
        tell every desktop
            set picture to "/Users/filepath/APOD{date}.jpg"
        end tell
    end tell'''
    p = Popen(['osascript', '-'], stdin=PIPE, stdout=PIPE, stderr=PIPE, universal_newlines=True)
    stdout, stderr = p.communicate(script)

while True:
    time.sleep(43200)
    print('Running')
    setwallpaper()

Is there any more efficient way to do this? Is there a better way to get this to run every 12 hours? In addition, is it a good idea to create a new file for each time I change the wallpaper?

\$\endgroup\$

3 Answers 3

3
\$\begingroup\$

Even for a small script, it's good practice to make a free-standing file with a hashbang:

#!/usr/bin/env osascript

tell application "System Events"
    tell every desktop
        set picture to "/Users/filepath/APOD{date}.jpg"
    end tell
end tell

replacing {date} with, probably, a reference to argv.

In this case, though, it may be a better idea to drop both AppleScript and Python, and rewrite as a single Swift script calling NSWorkspace setDesktopImageURL.

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

To answer your questions specifically, yes, there are some ways to improve your script by splitting your code into multiple, testable functions - each doing its own unit of work:

  • a function which fetches the data: fetch_apod_data;
  • a function which downloads the image: download_image;
  • a function which sets the wallpaper: set_mac_wallpaper;
  • a function which updates the wallpaper: update_wallpaper;
  • and last but not least, a function which schedules the wallpaper update: schedule_wallpaper_updates

To improve upon your specific questions you could:

  • use the schedule library to manage the scheduling part;
  • use the tempfile library to manage the creation of a new file issue;

The code would look like this (not tested)

from subprocess import Popen, PIPE
import requests
import json
import schedule
import datetime
import os
from tempfile import NamedTemporaryFile
from typing import Dict, Any


def fetch_apod_data(api_key: str) -> Dict[str, Any]:
    """
    Fetch the Astronomy Picture of the Day (APOD) data from NASA's API.
    """
    url = f'https://api.nasa.gov/planetary/apod?api_key={api_key}'
    response = requests.get(url)
    response.raise_for_status()
    return response.json()


def download_image(url: str) -> str:
    """
    Download an image from the specified URL and save it to a temporary file.
    """
    image_data = requests.get(url).content
    with NamedTemporaryFile(delete=False, suffix=".jpg") as temp_file:
        temp_file.write(image_data)
        return temp_file.name


def set_mac_wallpaper(image_path: str) -> None:
    """
    Set the desktop wallpaper on a Mac using AppleScript.
    """
    script = f'''tell application "System Events"
        tell every desktop
            set picture to "{image_path}"
        end tell
    end tell'''
    
    p = Popen(['osascript', '-'], stdin=PIPE, stdout=PIPE, stderr=PIPE, universal_newlines=True)
    stdout, stderr = p.communicate(script)
    
    if stderr:
        print(f"AppleScript Error: {stderr}")
    else:
        print("Wallpaper updated successfully.")


def update_wallpaper(api_key: str) -> None:
    """
    Update the desktop wallpaper to the current Astronomy Picture of the Day.
    """
    try:
        apod_data = fetch_apod_data(api_key)
        image_url = apod_data['url']
        print(f"APOD URL: {image_url}")

        image_path = download_image(image_url)
        set_mac_wallpaper(image_path)
    except requests.exceptions.RequestException as e:
        print(f"HTTP Request Error: {e}")
    except json.JSONDecodeError as e:
        print(f"JSON Decode Error: {e}")
    except KeyError as e:
        print(f"Missing expected data in response: {e}")
    except Exception as e:
        print(f"Unexpected Error: {e}")


def schedule_wallpaper_updates(api_key: str, interval_hours: int) -> None:
    """
    Schedule the wallpaper update to run at a specified interval.
    """
    schedule.every(interval_hours).hours.do(update_wallpaper, api_key=api_key)

    update_wallpaper(api_key)
    
    while True:
        schedule.run_pending()
        time.sleep(60)


if __name__ == "__main__":
    api_key = 'your_api_key_here'
    schedule_wallpaper_updates(api_key, 12)

If you want to avoid the infinite loop, another way to schedule tasks could be to use system-level scheduling tools such as cron on Unix-based systems (like the one you're supposedly using).

\$\endgroup\$
1
  • \$\begingroup\$ Hm, are you sure about NamedTemporaryFile for something that actually needs to be persisted? As a user, I'd prefer my wallpaper to survive the reboot... And schedule is not stdlib - would be nice to indicate that in the answer. \$\endgroup\$
    – STerliakov
    Commented Jul 5 at 0:28
2
\$\begingroup\$

I'll try to complement an answer by @GrajdeanuAlex by a few more suggestions.

Scheduling

First, you are lucky: you develop this for a known platform, and that platform is not Windows. You have a multitude of different tools available out of the box.

Your current script tries to run infinitely. To improve its fault resistance against everything - from coding errors to system failures - you'll anyway need some external observer that keeps the script alive and restarts it on failure. Another answer suggested pip install schedule, but I'm not super impressed by that: you still need some external tool to keep this thing running. Let's ask MacOS tools (ready out of the box - nothing to install!) to do that for you: let me introduce launchd. You'll also find cron documented there, which is marginally easier to set up (one line vs an XML file), but is considered legacy for some reason. So, with either of these tools you can ask your OS: please launch file change_wallpaper.py at 2 am, for example.

You still need to consider what should happen if PC was off or asleep at the scheduled time. In general, you can't rely on it being executed as soon as PC is back to life. If you need good reliability, you can schedule the script to execute more often and record last invocation time somewhere. If it is recent enough, exit early, otherwise proceed to the main task.

File management

I'd suggest using one file forever. Why create new files every time, if you have no plans to use previous versions? It also solves the problem of recording previous run time: you know a file, and that file has mtime. OS will store that timestamp for you.

Now gray area: what happens if you replace a file that's used as a wallpaper? Does the system catch that itself? Does it crash trying to read the corrupted file mid-write? Does it just cache the content somewhere and ignore modifications? I don't know, I don't have anything MacOS nearby - consider this a review question:)

I'll assume that you can replace the file safely. If not, you could maintain two files (current and previous) for simplicity, and update one with earlier mtime.

And since it's one file forever, you can make it UNIX-way: create a folder for your utility and use it. I'd put it at ~/.local/share/nasa-wallpaper/ - my rule of thumb for user-specific files managed by utilities is to use ~/.config for configuration (should be backed up), ~/.cache for something non-persistent (can be cleaned by user at any time), and ~/.local/share/ for semi-persistent files (can be restored, but good to keep for normal operations).

So, building on top of previous answer, your script could start like this:

from __future__ import annotations

import logging
import subprocess
from datetime import UTC, datetime, timedelta
from pathlib import Path
# You may ditch the typing - probably overkill for a tiny script
from typing import Any, Final

import requests

APP_FOLDER: Final = Path("~/.local/share/nasa-wallpaper/")
IMAGE_PATH: Final = APP_FOLDER / "current.jpg"


def fetch_apod_data(api_key: str) -> dict[str, Any]:
    """Fetch the Astronomy Picture of the Day (APOD) data from NASA's API."""
    url = "https://api.nasa.gov/planetary/apod"
    response = requests.get(url, {"api_key": api_key}, timeout=5)
    response.raise_for_status()
    return response.json()


def download_image(url: str, destination: Path) -> None:
    """Download an image from the specified URL and save it to the file."""
    image_data = requests.get(url, timeout=10).content
    destination.write_bytes(image_data)


def set_mac_wallpaper(image_path: Path) -> None:
    """Set the desktop wallpaper on a Mac using AppleScript."""
    script = f"""tell application "System Events"
        tell every desktop
            set picture to "{image_path.resolve()}"
        end tell
    end tell"""

    subprocess.run(
        ["osascript", "-"],
        input=script,
        stdout=subprocess.DEVNULL,
        stderr=subprocess.PIPE,
        timeout=10,
        check=True,
        text=True,
    )


def should_run_now(interval_hours: int, reference_file: Path) -> bool:
    """Did enough time pass since last successful invocation?"""
    if not reference_file.exists():
        # Never ran before
        return True
    last_run_time = datetime.fromtimestamp(reference_file.stat().st_mtime, tz=UTC)
    current_time = datetime.now(tz=UTC)
    return last_run_time + timedelta(hours=interval_hours) <= current_time


def update_wallpaper(api_key: str, image_file: Path) -> None:
    """Update the desktop wallpaper to the current Astronomy Picture of the Day."""
    try:
        apod_data = fetch_apod_data(api_key)
    except requests.exceptions.RequestException:
        logging.exception("Failed to fetch API data")
        return
    except json.JSONDecodeError:
        logging.exception("Malformed API response")
        return

    try:
        image_url = apod_data["url"]
    except KeyError:
        logging.exception("Missing expected data in response")
        return

    try:
        download_image(image_url, image_file)
    except requests.exceptions.RequestException:
        logging.exception("Failed to retrieve the image")
        return

    try:
        set_mac_wallpaper(image_file)
    except (subprocess.TimeoutExpired, subprocess.CalledProcessError):
        logging.exception("Failed to change wallpaper")
        return

    logging.info("Wallpaper changed successfully.")


if __name__ == "__main__":
    # As a future exercise, storing API keys in code is not great.
    # You may want to add a file in `~/.config` to store it.
    # You can provide `--setup` flag to ask for API key and store it.
    api_key = "your_api_key_here"
    interval_hours = 12

    logging.basicConfig(level=logging.INFO)
    APP_FOLDER.mkdir(exist_ok=True, parents=True)

    if should_run_now(interval_hours, IMAGE_PATH):
        update_wallpaper(api_key, IMAGE_PATH)
    else:
        logging.info("Run skipped")

Several other changes I incorporated:

  • requests.get usually should be performed with timeout to protect against being randomly stuck.
  • requests.get supports params parameter, no need to pass API key in f-string which is error-prone - query parameters may need escaping
  • s/print/logging/ - what if I want to use a file output later? This also gives you nice tracebacks for free.
  • Split exception handlers: don't catch exceptions you do not expect. It may be appropriate for simple scripts, but can hurt when they grow.
  • s/subprocess.Popen/subprocess.run/ and add timeout: subprocess.run is a less flexible API, but it supports everything needed here and wraps most of the complications.
  • Finally, I ran ruff format and ruff check on that file. Consider also adopting ruff or black to format your code automatically, and linter such as ruff, flake8 or pylint to check for common problems.
\$\endgroup\$
2
  • \$\begingroup\$ Your comment about Windows seems kind of out of place - I don’t see how that makes the task particularly harder. This is a fine answer without that commentary \$\endgroup\$ Commented Jul 5 at 2:56
  • \$\begingroup\$ Great response, this'll really help when I update my code. I'm creating new files every time since it didn't work when I created one file. It decided not to update it. There's probably another way without creating new files and using up my storage, but I haven't found it. \$\endgroup\$
    – Astrovis
    Commented Jul 9 at 13:18

Your Answer

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

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