Terminal handling charges for different countries

Question

I have the following problem that I solved using Python (Numpy/Pandas) and the code is provided later on. I mainly program in Java and wrote this program for a job interview as Python developer. I eventually got rejected and got the feedback saying the effort is not satisfactory. So, I mainly look forward to the suggestions from the expert Python developers. Please, tell me what should I do to improve the program.

---

Terminal handling charges are charges for loading/offloading shipping containers in ports. The charges are characterised by the following attributes:

* port – expressed as a portcode, a 5-letter string uniquely identifying a port. Portcodes consist of 2-letter country code and 3-letter city code.
* supplier_id - integer, uniquely identifying the provider of the information
* currency - 3-letter string identifying the currency
* value - a floating-point number

However, all the terminal handling charges collected are to be considered as potentially erroneous as they are provided by untrustworthy sources and can hence contain various types of mistakes. It is therefore important to develop a method to validate and ensure the quality of the terminal handling charges.

Goal of the Exercise

Develop a method for inspecting the distribution of the terminal handling charges. Identify and mark outliers (potentially erroneous values) for exclusion from the utilized collection of charges.

Details

• We are providing a set of initial sample data in the exersise.
• It is sufficient to look at the data on a country level, since the amount of data for individual ports might not be sufficient.

Currencies

Charges can be in different currencies. All charges can be normalised to USD. Currency exchange information can be obtained from various sources, for example openexchangerates.com. It is safe to assume that the currency exchange can be calculated with the current day's rates.

System input and output

• The list of values per country should be visualised in a meaningful way, identifying the amount of good/bad charges present in the system.
• The system should be able to accept new entries, checking them with the developed algorithm, and storing them along with the existing data, marked either as ok or an outlier.

sample_data.json

[{
    "currency": "CNY",
    "supplier_id": 35,
    "port": "CNAQG",
    "value": 820.0
}, {
    "currency": "CNY",
    "supplier_id": 19,
    "port": "CNAQG",
    "value": 835.0
}, {
    "currency": "CNY",
    "supplier_id": 49,
    "port": "CNAQG",
    "value": 600.0
}, {
    "currency": "CNY",
    "supplier_id": 54,
    "port": "CNAQG",
    "value": 775.0
}, {
    "currency": "CNY",
    "supplier_id": 113,
    "port": "CNAQG",
    "value": 785.0
}]

constants.py

PATH_LOCATION = '../TerminalCharges/data'

SUPPORTED_CURRENCIES = {
    "EUR": "European euro",
    "USD": "US dollar",
    "GBP": "Pound sterling",
    "BRL": "Brazilian real",
    "CNY": "Chinese Yuann"
}

COUNTRIES = {
    'CN': 'CHINA',
    'US': 'USA'
}

CURRENCIES_DIC = {'CN':'CHINA', 'US':'USA'}

LOW_Q = 0.05
HIGH_Q = 0.95

API_LINK = "https://api.fixer.io/latest?base={}&symbols={}"

terminal.py

import json

from pprint import pprint
import pandas as pd

import numpy as np 
import requests 
import matplotlib.pyplot as plt

from os import listdir, walk
from os.path import isfile, join
from constants import * 



# get the exchange rate from the base 
#  to the target currencies 
def get_exchange_rate(base_currency, target_currency):

    if not (base_currency in SUPPORTED_CURRENCIES.keys()):
        raise ValueError("base currency {} not supported".format(base_currency))

    if not (target_currency in SUPPORTED_CURRENCIES.keys()):
        raise ValueError("target currency {} not supported".format(target_currency))

    if base_currency == target_currency:
        return 1

    api_uri = API_LINK.format(base_currency, target_currency)
    api_response = requests.get(api_uri)

    if api_response.status_code == 200:
        return api_response.json()["rates"][target_currency]


# get the data after startdard cleansing 
def get_cleaned_data(df):
    df = df[df.notnull().all(axis=1)]
    df.port = df.port.astype(str)
    df.supplier_id = df.supplier_id.astype(int)
    df.currency = df.currency.astype(str)
    df.value = df.value.astype(float) 

    m1 = (df.port.astype(str).str.len() == 5) & (df.port.apply(lambda x :isinstance(x, str)))
    m2 = df.supplier_id.apply(lambda x : isinstance(x, int))
    m3=(df.currency.astype(str).str.len() == 3)&(df.currency.apply(lambda x :isinstance(x, str)))
    m4 = df.value.apply(lambda x : isinstance(x, float))

    mask = m1 & m2 & m3 & m4
    df = df[mask] 

    row_count = df.shape[0]
    col_count = df.shape[1]

    return df 


# convert to other currencies 
# to the usd  
def normalize_to_usd(df):
    exchage_rate_to_usd = {}
    for curr in SUPPORTED_CURRENCIES.keys():
        exchage_rate_to_usd[curr] = get_exchange_rate(curr, 'USD')  
    for curr in SUPPORTED_CURRENCIES.keys():    
        df.loc[df.currency == curr, ['value']] = df['value']*exchage_rate_to_usd[curr]
        df.loc[df.currency == curr, ['currency']] = 'USD'
    df.value = df.value.round(2)
    return df


#  mark 10% of the data as outliers
def mark_outliers(df):
    df["country"] = df["port"].str[:2].map(CURRENCIES_DIC)
    df["outlier"] = df.groupby("country")["value"].transform(mark_outliers_helper)
    row_count = df.shape[0]
    col_count = df.shape[1]
    return df 


def mark_outliers_helper(series):
    lower = series < series.quantile(LOW_Q)
    upper = series > series.quantile(HIGH_Q)
    return lower | upper


# get the percentage of the outlier 
#  for the respective country 
def get_outliers_percentage(df):
    gb = df[['country','outlier']].groupby('country').mean()
    for row in gb.itertuples():
        print('Percentage of outliers for {: <12}: {:.1f}%'.format(row[0], 100*row[1]))



#  test the accumulative df for the dimensions 
def test_df_dimensions(df, row_count):
    n_matrix =  np.array(df)
    assert isinstance(n_matrix, np.ndarray) or \
    isinstance(n_matrix, np.matrixlib.defmatrix.matrix)

    # print n_matrix.shape, row_count, df.shape[1]
    assert n_matrix.shape == (row_count, df.shape[1]) 


def preprocess(dfs, functions):
    apply_rec = lambda f, d: f[0](d) if len(f) == 1 \
                else apply_rec (f[1:], f[0](d))  
    storage = [apply_rec (functions, df) for df in dfs]

    df = pd.concat(storage, ignore_index=True)
    return df    


def generic_file_reader(PATH_LOCATION):
    dfs = []
    row =  0
    file_names = next(walk(PATH_LOCATION))[2]   
    for file_name in file_names:
        if file_name.endswith('.json'):
            df = pd.read_json(PATH_LOCATION + '/' + file_name)  
            row += df.shape[0]
            # row += df.count()
        elif file_name.endswith('.csv'):
            df = pd.read_csv(PATH_LOCATION + '/' + file_name)  
            row += df.shape[0]
        else:
            print 'file type is not supported now'
        dfs.append(df)
    return dfs, row

def visualize(functions, df):

    for fun in functions:
        fun(df)


# visualize the data in the line 
#  graph with pivot table 
def visualize_line_graph(df):
    df = df[df.outlier != 1]
    df = pd.pivot_table(df, 
                     index='supplier_id', 
                     columns = df['country'],
                     values='value', 
                     aggfunc=np.mean, 
                     fill_value=0)

    df.CHINA = df.CHINA.round(2)
    df.USA = df.USA.round(2)
    df = df[(df['CHINA'] != 0) & (df['USA'] != 0)]
    df.plot()
    plt.show(block=True)


def visualize_box_plot(df):

    df = df[df.outlier != 1]
    df = pd.pivot_table(df, 
                     index=df.index, 
                     columns = df['country'],
                     values='value')

    #print (df.mask(df == 0))
    ##alternative solution
    #print (df.replace(0,np.nan))

    # df.mask(df == 0).plot.box()
    df.boxplot()
    plt.show()


def main():
    clean_up_funcs =  [get_cleaned_data, normalize_to_usd, \
        mark_outliers]
    vis_funcs =  [visualize_line_graph, visualize_box_plot]

    dfs, row_count = generic_file_reader(PATH_LOCATION)

    df = preprocess(dfs, clean_up_funcs)
    test_df_dimensions(df, row_count)

    get_outliers_percentage(df)
    visualize(vis_funcs, df)


if __name__ == "__main__":
    main()

Answer 1

1. In the get_exchange_rate, you code silently ignores the error if the response code is not 200 and returns None, which can lead to an expected exception later on. I'd suggest to raise an appropriate exception here (the simplest thing to do is to the standard request's raise_for_status method).

2. In the get_cleaned_data function, you cast the columns to a specific type and then check it. For example, you cast the port column to an str: df.port = df.port.astype(str) and then check that's an str: isinstance(x, str). It's useless because it's always a string if you cast to a string. I'd recommend to validate the data first (using standard functios like isnumeric to check that something is indeed a number), filter it to remove invalid rows and only then cast the result to an appropriate type (your current version will raise an exception if, say, the value column has a non-numeric value).

3. Before removing the outliers, you can also remove the data that makes no sense (like negative prices).

4. I don't see the point in using recursion in the preprocess function. For instance, this lambda function looks quite confusing: apply_rec = lambda f, d: f[0](d) if len(f) == 1 else apply_rec (f[1:], f[0](d)). I believe that a simple loop would be more readable here.

5. Joining to path as strings with a hardcoded separator might break if you code is run a different OS. You can use os.path.join for a more portable and robust solution.

6. I think that printing the filename itself here:

   else:
       print 'file type is not supported now'
   

   would be nice (otherwise, the user might not which file this message refers to).

7. In the visualize_box_plot function, you don't need to compare anything to one: df = df[df.outlier != 1]. df.outlier contains boolean values. You can use it directly like df[df.outlier].

8. Functions and methods are usually named with a verb. The generic_file_reader doesn't sound like a good function name to me (moreover, it provides little information what it actually does). I'd suggest calling it like read_all_files_in_dir or load_data_from_all_files_in_dir.

9. You code is lacking proper documentation. Your comments are quite terse and they don't make it clear what a specific function does. I'd suggest writing proper doc comments for all functions in your code to make it easy to see what each function is supposed to do and be more specific about it (for instance, if a function reads all files in a directory and loads the data, it would be reasonable to write what kind of input format it can handle and what it does if it fails to work with a file).

10. Your specification also says that the application is supposed to handle new data. I don't see that functionality in your code (as I can see, it just reads all the data in bulk and combines it before processing the data).

11. You could also make you system more flexible by trying different ways of outlier detection (at very least, you could make your filtering function take LOW_Q and HIGH_Q as arguments).

To sum it up, I'd focus on the following aspects:

1. More robust error handling.

2. Clearer variable naming and documentation.

3. Making sure that it follows all specification, including adding new data to an existing dataset (you might also try more advanced outlier detection techniques here).