3
\$\begingroup\$

This is my first real excursion into Python, coming from C#. I wanted to see if my code is well-formatted and efficient.

The following will read the needed codes from a Database Table, then submit the codes to an API, store them in individual objects, and then dump them back into another table in the Database.

import ondemand
import pyodbc
import datetime

#Code purposes
#1.) Get the desired barchart codes for the year
#2.) Load quotes into an array from Barchart API
#3.) Submit quotes to database for today's date

#Qute Class - Stores the quotes as objects
class Quote():
    def __init__(self, seq_dimDate, symbol, month, year, price, crop, ficalYear):
        self.seq_dimDate = seq_dimDate
        self.symbol = symbol
        self.month = month
        self.year = year
        self.price = price
        self.crop = crop
        self.fiscalYear = ficalYear


#Establish a connection to the database
cnxn = pyodbc.connect('DRIVER={SQL Server};SERVER=MyComputer\\SQLEXPRESS;DATABASE=MarketDB')
cursor = cnxn.cursor()

#get the Date and Fiscal Year
now = datetime.datetime.now().strftime('%m/%d/%Y')
dateRow = cursor.execute('SELECT * from dbo.dimDate WHERE [Date] = ?', now).fetchone()
seqDimDate = dateRow.Sequence
fiscalYear = dateRow.FiscalYear
print('Sequence for %s is %s with a fiscal year of %s' % (now, seqDimDate, fiscalYear))

#Query the Barchart Codes Table
cursor.execute('SELECT * FROM dbo.Codes WHERE [Year] = %s' % (fiscalYear))


#Generate a list of codes to send to API
quotesToQuery = ''

for row in cursor.fetchall():
    for x in range(3,len(row)):
        quotesToQuery = quotesToQuery + ',' + row[x]


#Open connection to the API
od = ondemand.OnDemandClient(api_key='123', end_point='https://marketdata.websol.barchart.com/')

#Get quotes
quotes = od.quote(quotesToQuery)['results']
lQuotes = []

#load the quotes into our array of objects
for q in quotes:
    #Get the Symbol, month, year and price of the quote
    symbol = q['symbol']
    dt = datetime.datetime.strptime(q['tradeTimestamp'], '%Y-%m-%dT00:00:00-%H:%M')
    month = dt.month
    year = dt.year
    price = q['lastPrice']

    #Get the Sequence for the Crop
    if q['name'] == 'Corn':
        crop = 1
        price = price / 100
    elif q['name'] == 'Soybean Meal':
        crop = 2
    else:
        crop = 3

    #create a new Quote object and add it to the collection
    nq = Quote(seqDimDate, symbol, month, year, price, crop, fiscalYear)
    lQuotes.append(nq)

#Double Check of Quotes
print('%s total Quotes Collected' % (len(lQuotes)))

#Delete any Prices in the Database for this date sequence
cursor.execute('DELETE FROM dbo.MarketPrice WHERE Seq_dimDate = ?', seqDimDate)
cursor.commit()
print('MarketPrice table cleared of records with Date Sequence %s...' % (seqDimDate)) #confirm records were deleted

#Submit the Quotes to the Database
for n in lQuotes:
    cursor.execute('INSERT INTO dbo.MarketPrice (Seq_dimDate, Seq_Crop, [Month], FiscalYear, Price, Comments) VALUES(?,?,?,?,?,?)', n.seq_dimDate, n.crop, n.month, n.fiscalYear, n.price, n.symbol)
    cursor.commit()
    print('%s Uploaded to MarketPrice Table in Hedging2' % (nq.symbol)) #confirm update

I submit myself to your critique!

\$\endgroup\$
1
\$\begingroup\$

Python review

This could make better use of docstrings and functions; for example, your comment about the Quote class should be a well-formatted docstring instead. Additionally, each of your different comment blocks would be best suited as a function, and then piece them together inside of a __main__ block.

I would also do my best to remove any of your magic values, and instead use constants. This makes it easier if they come from somewhere else (e.g. a config file, or somewhere on the database) to adjust.

When formatting, prefer str.format over % formatting. If you're on a high-enough Python version, f-strings are even nicer.

This line is very magic: for x in range(3,len(row)). I would strongly prefer that you document what is going on here using non-magic numbers (and maybe a code comment). I think that what you're doing is getting the value from every column after the first 4; if that is what you're doing, then

  1. Improving the SQL query is good to do here (see #1 in the SQL review below)
  2. There are better variable names to use here

Generators are a good thing to use, as are list comprehensions. You can also use tuple-unpacking to make trivial assignments easier.

Instead of iteratively building a string, which performs poorly, use str.join. Similarly, instead of repeatedly list.appending, use list.extend or a list comprehension.

Instead of cascading your if/elif/else, use a dictionary and a default value.

SQL review

In general, your SQL is pretty straightforward so there isn't a ton to say. Overall, you should:

  1. When SELECTing something, only get what you need. SELECT * should always be considered suspect.
  2. Avoid embedding raw SQL in your source code; if the desired behavior ever changes, now you're needing to update this code for no reason. A stored procedure (or table-valued function) is likely going to be a better option.
  3. Always use your language/libraries tools for parameterizing SQL; you seem to switch back and forth between this and using string formatting. Always remember that no input is trusted, even if it comes from your own database.
  4. Instead of inserting VALUES row-by-row, investigate if there is a bulk INSERT option for your library. If not, consider other options (as row-by-row is going to perform poorly)
  5. Consider deleting and inserting in a single commit so as to avoid leaving the database in an unusual state in-between operations.

Updated code

I came up with the following updates (that assume you have created the appropriate stored procedures, and I didn't bother with fully making all literals constants). I think there are a few more places for improvement here; for example, some of the formatting you do before instantiating the class could be done in the class itself (for example, a property that returns the formatted value, or a setter on that property that sets it in a formatted way). I also think you could likely break this up into a few more classes if you really wanted to make it OOP, but that is probably overkill for your use-case. """ Gets barchart codes for a given year, then loads them into an array using BarChart's API. From there, the quotes are pushed to the database using the current date """

import datetime

import ondemand
import pyodbc


CONNECTION_STRING = "DRIVER={SQL Server};SERVER=MyComputer\\SQLEXPRESS;DATABASE=MarketDB"
API_KEY = "123"
END_POINT = "https://marketdata.websol.barchart.com/"
NAME_TO_CROP_MAPPING = {
    "Corn": (1, lambda price: price / 100),
    "Soybean Meal": (2, lambda price: price)
}
DEFAULT_CROP_MAPPING = (3, lambda price: price)


class Quote():
    """Stores the quotes as objects"""

    def __init__(self, seq_dimDate, symbol, month, year, price, crop, ficalYear):
        self.seq_dimDate = seq_dimDate
        self.symbol = symbol
        self.month = month
        self.year = year
        self.price = price
        self.crop = crop
        self.fiscalYear = ficalYear


def get_today_formatted():
    return datetime.datetime.now().strftime("%m/%d/%Y")


def get_sequence_date(cursor):
    now = get_today_formatted()
    row = cursor.execute("EXECUTE dbo.RetrieveDateAndFiscalYear @dateFilter = ?", now).fetchone()
    return (now, dateRow.Sequence, dateRow.FiscalYear)


def get_codes(cursor, fiscal_year):
    FIRST_CODE_INDEX = 3

    all_codes = cursor.execute("EXECUTE dbo.RetrieveBarchartCodesByYear = ?", fiscal_year)    
    for row in all_codes.fetchall():
        for code in range(FIRST_CODE_INDEX, len(row)): 
            yield row[code]


def get_date_from_timestamp(timestamp):
    date = datetime.datetime.strptime(timestamp, '%Y-%m-%dT00:00:00-%H:%M')
    return (date.month, date.year)


def get_quotes(od_connection, seq_dim_date, fiscal_year, quotes_to_query):
    raw_quotes = od_connection.quote(quotes_to_query)["results"]

    for raw_quote in raw_quotes:
        symbol = raw_quote["symbol"]
        month, year = get_date_from_timestamp(quote["tradeTimestamp"])
        base_price = q["lastPrice"]

        crop_id, adjusted_price = get_crop_sequence(q["name"], base_price)

        yield Quote(seq_dim_date, symbol, month, year, adjusted_price, crop_id, fiscal_year)


def get_crop_sequence(crop, base_price):
    crop_id, price_adjustor = NAME_TO_CROP_MAPPING.get(crop, DEFAULT_CROP_MAPPING)
    return (crop_id, price_adjustor(base_price))


def update_invalid_prices(cursor, seq_dim_date, quotes):
    cursor.execute("EXECUTE dbo.DeleteHistoricalPricesByDate @Date = ?", seq_dim_date)

    for quote in quotes:
        cursor.execute("EXECUTE dbo.UpdatePricesForQuote <<Fill in your parameters here>>", <<Fill in your parameters here>>)

    cursor.commit()

    print(f"MarketPrice table cleared of records with Date Sequence {seq_dim_date}...")
    print("\n".join([f"{q.symbol} Uploaded to MarketPrice Table in Hedging2" for q in quotes]))

if __name__ == "__main__":
    sql_connection = pyodbc.connect(CONNECTION_STRING)
    cursor = sql_connection.cursor()

    now, seq_dim_date, fiscal_year = get_sequnce_date(cursor)
    print(f"Sequence for {now} is {seq_dim_date} with a fiscal year of {fiscal_year}"))

    quotes_to_query = ",".join(get_codes(cursor, fiscal_year))
    ondemand_connection = ondemand.OnDemandClient(api_key=API_KEY, end_point=API_END_POINT)    
    all_quotes = list(get_quotes(ondemand_connection, seq_dim_date, fiscal_year, quotes_to_query))    
    print(f"{len(all_quotes)} total Quotes Collected")

    update_invalid_prices(cursor, seq_dim_date, all_quotes)
| improve this answer | |
\$\endgroup\$

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.