Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

homework19.1 #27

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 114 additions & 0 deletions lesson_19/homework19_1.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
"""
Homework 19.1 Mars Rover “Curiosity” photos.

NASA has an open API that allows retrieving data in JSON format
about photos taken by the “Curiosity” rover on Mars
based on specific parameters.
Among this data, there are links to photos that need to be parsed
and then downloaded and saved as local files
(mars_photo1.jpg, mars_photo2.jpg, etc.) using additional requests.
The task should be implemented using the requests module.
"""

import logging

import requests
from requests.exceptions import HTTPError, Timeout

# Configure logging
logging.basicConfig(
level=logging.INFO,
format='%(asctime)s - %(levelname)s - %(message)s',
)
_log = logging.getLogger(__name__)


class NasaImageParser:
"""
A class discribed a parser for NASA images.

Attributes:
url (str): NASA API link.
params (dict): dict of the params.
"""

def __init__(self, url: str, params: dict):
"""Initialize a new NasaImageParser with parameters."""
self.base_url = self._clean_url(url)
self.params = self._prepare_params(params)

@staticmethod
def _clean_url(url: str) -> str:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks redundant too - just use url from the beginnig without < and > signs

"""Clear the URL if additional chars in the start/end of the string."""
if url.startswith('<') and url.endswith('>'):
return url[1: -1]
return url

def _prepare_params(self, params: dict) -> dict:
"""Check if all required params are available."""
if not isinstance(params, dict):
raise ValueError('Params must be a dictionary.')
if 'api_key' not in params:
raise ValueError("The 'api_key' parameter is required.")
return params
Comment on lines +47 to +53
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks redundant for this task - lets remove


def get_photos(self):
"""Get the photos from remote."""
try:
response = requests.get(
self.base_url,
params=self.params,
timeout=10,
)
response.raise_for_status()
data = response.json()
return data.get('photos', [])
except (HTTPError, ConnectionError, Timeout) as err:
_log.error(f'Error fetching photos: {err}')
return []
Comment on lines +57 to +68
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have some logic for get request execution. This logic we use twice - > here and in photo download. So better to create helper function aka get_request and use it in both places


def download_photos(self, photos=None):
"""Download a single or all photos."""
# If photos not available
if photos is None:
photos = self.get_photos()
if not photos:
_log.info('No photos available for the given parameters.')
return # will close the func in case of no photos

# Fail downloads counter
failed_downloads = 0
for idx, photo in enumerate(photos, start=1):
photo_url = photo.get('img_src')
if not photo_url:
print(f"Photo {idx} has no valid 'img_src'. Skipping.")
failed_downloads += 1
continue

filename = f'mars_photo{idx}.jpg'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks as naming template . we could use it as constant or class attribute and then fill with format() call

try:
# Download the photo
with requests.get(photo_url, stream=True, timeout=10) as resp:
resp.raise_for_status()
with open(filename, 'wb') as fh:
for chunk in resp.iter_content(chunk_size=8192):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All hardcodes aka chunk_size, timeout etc better to move to init

fh.write(chunk)
_log.info(f'Downloaded: {filename}')
except (HTTPError, ConnectionError, Timeout) as err:
_log.error(f'Error downloading {photo_url}: {err}')
failed_downloads += 1

# End message
if failed_downloads:
_log.info(f'{failed_downloads} photo(s) could not be downloaded.')
else:
_log.info('All photos downloaded successfully!')
Comment on lines +101 to +105
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These logic should be hidden to main block below. Failued downloads could you could get as function call return value



url = '<https://api.nasa.gov/mars-photos/api/v1/rovers/curiosity/photos>'
params = {'sol': 1000, 'camera': 'fhaz', 'api_key': 'DEMO_KEY'}

if __name__ == '__main__':
downloader = NasaImageParser(url, params)
photos = downloader.get_photos()
downloader.download_photos()