cohaolain / ryanair-py

This is an open source Python module which allows you to retrieve the cheapest flights, with or without return flights, within a given set of dates.
https://github.com/cohaolain/ryanair-py
MIT License
51 stars 15 forks source link

Issue with currency settings in Ryanair package #2

Closed danielbich closed 1 year ago

danielbich commented 1 year ago

It appears that the package does not adhere to the specified currency setting for fetching flight prices (api = Ryanair("EUR")) and retrieves prices in various other currencies. I've noticed this issue while using the get_all_flights function.

cohaolain commented 1 year ago

Hi, just to confirm I've been able to reproduce this, I'll look into fixing this soon, thanks!

cohaolain commented 1 year ago

This may be a little tricky, the availability API used by get_all_flights doesn't seem to support specifying a currency. I suppose this is because if a user is booking a one-way flight, they are made to pay for their flight in the currency of the country they're departing from. We must also factor in that return flights will then likely have both legs required to be in the currency of the original departing country.

In fact, even for the get_cheapest_flights and get_cheapest_return_flights_ calls for which we pass a currency param to the API, I can see that this param seems to no longer be in use on the website (even though it still appears to work in this case, for now!). Those prices don't appear quite round though (e.g. €14.99 flight from an EUR country is stated to cost £13.17 if GBP is specified), so I'd speculate they've always just used an exchange rate for that stuff internally - even if it's not an actual sale price - you'd later be given the price in the denomination of the country you're departing from.

Perhaps at this stage, I should look at handling the currency conversions internally, if the provided currency doesn't match the response from the API... I could also get rid of the currency parameter altogether and leave this to the user, and pass up the response currency in the Flight and Trip objects instead :thinking:

danielbich commented 1 year ago

It seems to me that a functionality where, in addition to the price, the script would also fetch the currency in which the price is represented would be sufficient. If you could add such functionality, that would be great.

dolohow commented 1 year ago

I wrote this simple code to convert currencies


import shelve
import logging

def cache(func):
    def wrapper(*args, **kwargs):
       with shelve.open('cache.db') as db:
          cache_key = f"{func.__name__}{args}{kwargs}"
          if cache_key in db:
              cache_time, cache_data = db[cache_key]
              if time.time() - cache_time < 24 * 3600:
                  logging.debug(f"{func.__name__} - cache hit")
                  return cache_data
          logging.debug(f"{func.__name__} - cache miss")
          result = func(*args, **kwargs)
          db[cache_key] = (time.time(), result)
          return result
    return wrapper

@cache
def fetch_exchange_rate(base, target):
    logging.debug('Fetching exchange rate for %s/%s', base, target)
    # get the API key from https://www.exchangerate-api.com/
    API_KEY = "YOUR_API_)KEY"

    url = f"https://v6.exchangerate-api.com/v6/{API_KEY}/pair/{base}/{target}"

    response = requests.get(url)
    if response.status_code == 200:
        data = response.json()
        return data['conversion_rate']
    else:
        logging.error("Could not fetch exchange rate")

def convert_currency(base, amount):
    exchange_rate = fetch_exchange_rate(base, "PLN")
    converted_amount = round(amount * exchange_rate, 2)
    return converted_amount
cohaolain commented 1 year ago

It seems to me that a functionality where, in addition to the price, the script would also fetch the currency in which the price is represented would be sufficient. If you could add such functionality, that would be great.

@danielbich I'm making these changes now :) took slightly longer than expected to get a few minutes to hit commit on my work!

@dolohow

I wrote this simple code to convert currencies

This seems reasonable for people to use in the meantime if they stumble on this thread, thanks! I'm hoping to implement something similar internally to the library if the user has specified a currency - I might try and see if I can do this without requiring an API key to be configured, perhaps, and to update the cache internally at some interval to avoid cache staleness.

cohaolain commented 1 year ago

I've released a new version v2.2.0 which should make the currency response available, and provide a warning if it doesn't match what you've specified when creating the library instance.

If you don't want the warnings, it is now optional to provide currency when creating the library instance. So if you want to do the currency handling yourself across the board you can simply omit this and always check the results yourself :)

I may try and implement some basic currency conversion if the currency is specified in the future - we'll see!

Feel free to send any feedback or comment here if any issues arise on this topic.