Skip to content

test#14

Open
deliarae wants to merge 1 commit intoalexnaylor99:mainfrom
deliarae:branch
Open

test#14
deliarae wants to merge 1 commit intoalexnaylor99:mainfrom
deliarae:branch

Conversation

@deliarae
Copy link

Not sure if I'm doing this correctly

# IFTTT Webhooks credentials, for now
event_name = ""

# Calculate the start and end dates in YYYY-MM-DD format for the 7-day period,

Choose a reason for hiding this comment

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

character limit for comments is 72

tickers = ["TSLA", "AAPL", "MSFT", "GOOG", "NKE"]
response = {}
for ticker in tickers:
url = f"https://api.twelvedata.com/time_series?symbol={ticker}&interval=1day&start_date={start_date}&end_date={today}&apikey={api}"

Choose a reason for hiding this comment

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

character limit for code is 79

"NKE": "NKE_Price_Drop"
}

# Converting £0.25 to USD with current exchange rate
Copy link

@Arjun-berko Arjun-berko Oct 16, 2023

Choose a reason for hiding this comment

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

This is a valid and smart idea for currency conversion, simply converting £0.25 to USD, and going through all the conditionals in dollars. Although, converting all the API data from USD to GBP would be better for the client, as you can then send them stock data in GBP through IFTTT.

data_realtime = get_realtime_price(config.api_key2)


# --------- MARKET OPEN PRICE -----------

Choose a reason for hiding this comment

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

Very good comment structure

tickers = ["TSLA", "AAPL", "MSFT", "GOOG", "NKE"]
response = {}
for ticker in tickers:
url = f"https://api.twelvedata.com/price?symbol={ticker}&apikey={api}"

Choose a reason for hiding this comment

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

You can add error handling when making API requests, I didn't do that for my code either, but its worth looking into


# Calculate the start and end dates in YYYY-MM-DD format for the 7-day period,
# with end_date being yesterday and start_date being 7 days ago.
today = date.today()

Choose a reason for hiding this comment

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

Maybe put the finding of start dates and end dates in a function, that would increase clarity

import config
from datetime import date, timedelta


Copy link

@Arjun-berko Arjun-berko Oct 16, 2023

Choose a reason for hiding this comment

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

In general your code is very well organised and clear to read, with the way you've added comments at the start of each block and separated them out

# --------- TRIGGER WEBHOOK IF CONDITION IS MET -----------
# Function to trigger IFTTT event
def trigger(event_name):
url = f'https://maker.ifttt.com/trigger/{event_name}/with/key/{config.ifttt_key}'
Copy link

@Arjun-berko Arjun-berko Oct 16, 2023

Choose a reason for hiding this comment

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

You have your secret keys in a config file, which is good practice, something I wish I'd done.
Another suggestion for when you build programmes that are only meant to run on your computer are environment variables.

if current_price <= market_open_price - pence_to_cents:
trigger(event_name)
elif current_price < average:
trigger(event_name)

Choose a reason for hiding this comment

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

You need an if name == main block at the end. Any code inside this block is what gets executed whenever you run this file in the terminal

if current_price <= market_open_price - pence_to_cents:
trigger(event_name)
elif current_price < average:
trigger(event_name)

Choose a reason for hiding this comment

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

Also you could've written some tests. For example, to check:

  • if data is being fetched from the API correctly as required
  • if the function calculating the weekly average is working as required

else:
print(f"Failed to trigger the IFTTT applet for {event_name}.")

# Map of tickers to event names

Choose a reason for hiding this comment

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

Using dictionaries to map the stock names to their event names is such a smart idea, really centralises all event handling. Wish I'd though of that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants