8
\$\begingroup\$

This is my first script that I've written. As a result, I'm sure there are extra lines that are unneeded, or maybe better more concise ways of doing things than I have done here. I have tried to add explanations to each section so that it's a bit easier to understand what's going on, and why I'm doing what I'm doing.

Just a bit of backstory to explain what this script is designed to do. This script is designed to take in the output files from a specific program that is used to analyze we'll call "Titration Data", and to extract the data from these files and do some modifications to them. Once they've been modified, it then tries to fit the data to a model, and output the graphs, fit value itself, standard deviation, goodness of fit, and the output of the function (so someone can graph it in another program if they want). There are only 3 input files that are needed: The data files themselves, the concentration file, and the dilution file. The dimensions for all of these need to match.

That said, below is my script, any advice/feedback is greatly appreciated! If there is anything unclear, please let me know and I can clear it up. Thank you!

#I plan on adding other scripts later to analyze different data sets. Currently I have written the script for one. But this is designed to be a general use program, so I want to get it out there now, but add features to it later. Hence all the initial inputs.
A=input('Slow or Fast Exchange? (Type in "Slow" or "Fast")\n')
if A == ('Slow').lower():
    B=input('Will you be using both the free and bound state peaks? (Type in "Yes" or "No")\n')
    if B==('Yes').lower():
        print ('Slow Exchange analysis using both states is not developed yet')
    else:
        C=input('Will you be using the free or bound state peaks? (Type in "Free" or "Bound")\n')
        if C==('Free').lower():
            print ('Loading up Slow Exchange Script')
            import re
            import os
            import pandas as pd
            import numpy as np
            import matplotlib.pyplot as plt
            from scipy.optimize import curve_fit
#This is where the user inputs where the files they will be calling are stored. To make this more general user friendly, I automatically fill the drive and user portion
            main_path=(r'C:\Users\Sams PC')
            next_path=input('Please indicate the folder the files are. E.g. /Desktop/Peaklists \n')
            full_path=main_path+next_path
            os.chdir(full_path)
#The data that will be analyzed is split into 3 sections. One is the experimental data itself (Titration_files), the settings they used to obtain the experimental data (Concentrations), and a normalization factor (Dilution)
            Titration_files = []
            Data_Table = []
            Peak_Height = []
            Titration_file_input = ''
            Peak_Names=[]
            Dilutions=input('Enter name of Dilution file.\n')
            Concentrations=input('Enter name of Concentration file.\n')
# This is designed so the user can input in as many files as they desire. And if they misspell, or accidently add an extra file or forget a file, the script will still continue.
            print('Enter name of peaklist files. When finished, type done and enter to stop.')
            while True:
                    Titration_file_input = input()
                    if Titration_file_input.lower() not in ['done']:
                            Titration_files.append(Titration_file_input)
                    else:
                        break

            for Data in Titration_files:
                try:
                    Titration_Datatable = pd.read_csv(Data, sep='\s+', header=None)
                    Titration_Datatable.columns=['Column_1','Column_2','Column_3', 'Column_4', 'Column_5']
                    Data_Table.append(Titration_Datatable)
                except:
                    print('File' + ' ' + Data + ' ' + 'not found')

#The input of Titration_Data comes from a specific program that has the required data stored in Column 4. I am creating a second table from Column 1 to use when saving the plots and savefiles.
            for Titration_Datatable in Data_Table:
                Peak_Height.append(Titration_Datatable.loc[:,'Column_4'])
                Peak_Names.append(Titration_Datatable.loc[:,'Column_1'].drop([0],axis=0).drop([1],axis=0))
#The first row in the datatable automatically is labeled Data, so that is removed. The 2nd row also has miscellanious info. The .astype is because I had to define them as integers, otherwise I would get errors at the division step below. 
            concatenated_Titration_Datatable = pd.concat(Peak_Height, axis=1).drop([0],axis=0).drop([1],axis=0).astype(int)
            Dilutions=pd.read_csv('Dilution.txt', sep='\s+', header=None)
#The Data may sometimes have negative values, which can be considered the same as zero in this case.
            Combined=concatenated_Titration_Datatable.clip(lower=0)
#The Data now needs to be normalized to account for Dilution (this is introduced in the experimental setup), the values are defined by a txt file the user uploads.
            Normalized=(Combined/Dilutions.values)
#It appears easier to do data modifications with a numpy matrix rather than with pandas, so I converted this to numpy. I also had to do it for the function below as well.
            M=pd.DataFrame.to_numpy(Normalized)
#The below function is part of Data processing. It's an equation for analyzing the data.
            J=(M[:, :1]-M)/((M[:, :1]-M)+ M)

            Titration_Data=J
            Concentrations=np.loadtxt('Concentrations.txt')
            Protein=Concentrations[:,0]
            Ligand=Concentrations[:,1]
            Input_Data=[Protein,Ligand]
            x=Input_Data[1]
            A=Input_Data[0]
            B=Input_Data[0]+Input_Data[1]
            C=Input_Data[1]
#To be able to save the Peak_Names list above as a png for the graphs below, I had to convert to a nupy array first. But this would cause errors since its a matrix, thus I removed all the repeats, giving me a 1D array.
            Peak_Names_ar=np.array(Peak_Names)
            Peak_Names_array=np.unique(Peak_Names_ar)
#I have setup the values A,B, and C because it makes the below equation cleaner and easier. The only reason x is defined is because I couldn't run the function without it, although it doesn't appear to effect the output.
            def fun(x, kd):
                return np.array((B+kd-np.sqrt(((B+kd)**2)-4*A*C))/(2*A))
#For each iteration of the function, I want to save the output of the fit (kD), the standard deviation of that fit, and R2 (goodness of fit). I'm also saving each plot, and using the above Peak_Names file to do so.
            kD=[]
            r2=[]
            standard_deviation=[]
            output_for_graphing=[]
            for values,i in zip(Titration_Data,Peak_Names_array):
                Intensity=[values]
                Intensity_Array=np.array(Intensity)
                y=Intensity_Array.flatten()
                popt, pcov = curve_fit(fun, x, y)
                kD.append(popt)
                fun_data=fun(x,*popt)
                output_for_graphing.append(fun_data)
                residuals=y-fun(x, popt)
                ss_res=np.sum(residuals**2)
                ss_tot=np.sum((y-np.mean(y))**2)
                r_squared=1-(ss_res/ss_tot)
                r2.append(r_squared)
                std = np.sqrt(np.diag(pcov))
                standard_deviation.append(std)
                plt.plot(x, y, label='data')
                plt.plot(x, fun(x, *popt), label='fitted')
                plt.xlabel('Ligand Concentration')
                plt.ylabel('Intensity')
                plt.title([i])
                plt.grid()
                plt.legend()
                files_to_save=str([i])+'.png'
                plt.savefig(files_to_save)
                plt.show()
#I'm saving the output of the function as well, in case someone wants to graph it for themselves and doesn't like matplotlibs display.
            np.savetxt('Output for graphing.txt',output_for_graphing)
#I need to flatten the arrays to be able to stack them (I get an error otherwise)
            kD_array=np.array(kD).flatten()
            r2_array=np.array(r2).flatten()
            standard_deviation_array=np.array(standard_deviation).flatten()
            Dissociation_Constant=np.stack((kD_array,standard_deviation_array,r2_array),axis=-1)
# I realized after the fact that I wanted to add an extra column to the above matrix. But since numpy doesn't seem to have an insert command, and I didn't want to break the matrix I already had, I thought it might be easier to just change it to a pandas datatable.
            Dissociaton_Constant_Table=pd.DataFrame(Dissociation_Constant)
            Dissociaton_Constant_Table.columns=['kD', 'Standard Deviation', 'R2']
            Dissociaton_Constant_Table.insert(0,'Files',Peak_Names_array)
            Dissociaton_Constant_Table.to_csv('Dissociation_Constant.txt', sep='\t', index=False)
        else:
            print ('Slow exchange analysis for the bound peak is not developed yet')

Input Files (incase anyone wanted to run it themselves):

#Concentrations 
0.6     0.06
0.596421471 0.119284294
0.5859375   0.29296875

#Dilution
1.03E+05    1.03E+05    9.50E+04    

#Data file 1
      Assignment         w1         w2   Data Height 

        Dil_XN-HN    122.352      7.682       102832 
           1XH-HN    111.783      9.206        50034 
           2XN-HN    104.093      9.040        38230 
           3XN-HN    107.595      7.443        34503 
           4XN-HN    106.481      8.252        34219 
           5XN-HN    108.303      8.396        37442 
           6XN-HN    112.540      9.481        32144 
           7XN-HN    132.030      9.051        28053 
           8XN-HN    128.895      8.000        30395 
#Data file 2
      Assignment         w1         w2   Data Height 

        Dil_XN-HN    122.352      7.682       102832 
           1XH-HN    111.771      9.206        50034 
           2XN-HN    104.093      9.040        38230 
           3XN-HN    107.595      7.443        34503 
           4XN-HN    106.481      8.252        34219 
           5XN-HN    108.303      8.396        37442 
           6XN-HN    112.540      9.481        32144 
           7XN-HN    132.030      9.051        28053 
           8XN-HN    128.895      8.000        30395 
#Data file 3:
      Assignment         w1         w2   Data Height 

        Dil_XN-HN    122.348      7.679        95020 
           1XH-HN    111.738      9.202        32355 
           2XN-HN    104.118      9.037        31054 
           3XN-HN    107.619      7.438        22208 
           4XN-HN    106.516      8.252        22975 
           5XN-HN    108.300      8.397        25319 
           6XN-HN    112.506      9.484        21293 
           7XN-HN    132.037      9.047        19972 
           8XN-HN    128.906      7.992        22899 
```
\$\endgroup\$
  • 4
    \$\begingroup\$ A, B, C are bad names. don't do that. \$\endgroup\$ – Alexander - Reinstate Monica Dec 22 '19 at 16:16
  • \$\begingroup\$ Are you saying for the input files at the begining or for my function variables later on? If it's for just the inputs, I do plan to make those something more understandable later (they're just placeholders currently since the main script is what's written below them) \$\endgroup\$ – samman Dec 22 '19 at 16:21
  • 5
    \$\begingroup\$ Samman having to ask that question perfectly exemplifies why you shouldn't use A, B and C as variable names :) There wouldn't be ambiguity if they were clearly named. \$\endgroup\$ – Alexander - Reinstate Monica Dec 22 '19 at 16:32
  • 1
    \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Simon Forsberg Dec 23 '19 at 0:00
10
\$\begingroup\$

I've never used numpy or matplotlib, so I can only speak to issues of style.

You're allowing for far too much nesting here. Your code consists of a giant, dense, deeply nested chunk. As a result, the eyes have very few good landmarks to rest on; making your code hard to read.

Put your existing function definitions at the very top level outside of any nesting, and try to break the rest of the code down into more "bite-size" functions. Code is much easier to understand when it's broken down into logical parts that each have a clearly defined purpose. With how you have it now, it's difficult to make sense of any single part without having a good understanding of the whole since it isn't clear what precisely any given piece of code is doing, and where the job of one chunk of code ends and another begins.


Your imports should always be at the very top unless you have a good reason to have them lower, like importing is abnormally expensive and only conditionally necessary, or you need to conditionally choose between several imports due to the platform being used or something similar.

The entirety of your code seems to be everything under the if C==('Free').lower(): condition though, so every import will always be needed.


I think your comments should be at the same level of indentation as the code they're commenting on. PEP8 doesn't seem to say anything on the matter, but I find your comments confusing to read with how you have it now. If they're formatted as they are to prevent lines from getting too long, I'd work on the nesting issue first as I mentioned above. Fixing nesting generally tends to fix a lot of other readability problems with it.


I'm not sure if this in an artifact from something that was changed, but this:

if Titration_file_input.lower() not in ['done']:

Can just be

if Titration_file_input.lower() != 'done':

Your names are all in Upper_Snake_Case. Regular variables names should be in lower_snake_case though.


Your spacing around binary operators is inconsistent:

if B==('Yes').lower()

. . .

Titration_file_input = ''
Peak_Names=[]

I always have one space on either side of all binary operators, regardless of context; that's my personal preference. Whatever you choose though, you should be consistent. Inconsistency is another thing that generally tends to harm readability.


You can use f-strings to neaten up some parts. For example:

print('File' + ' ' + Data + ' ' + 'not found')

Can be written as:

print(f'File {Data} not found')  # Note the f', similar to r'

Which is significantly neater.


Titration_Data=J

I wouldn't do this. The only purpose this has is to give a second name to J. If you find that to be necessary though, I would just not call it J in the first place:

Titration_Data=(M[:, :1]-M)/((M[:, :1]-M)+ M)
\$\endgroup\$
  • \$\begingroup\$ Thanks for the feedback, I've implemented some of the shortcuts to make it look more concise. In regards to the nesting, capitalization, and comment indents. I have everything being defined as per basis, i.e. the variable is defined right before it's used rather than defining it early on (I wrote it in this manner because it helped me understand what that variable is being used for, and why I have it there). I capitalize the first of every word because I actually find it easier to read than all lower case ironically (but I can change all those), and the lack of indentation for the comments is \$\endgroup\$ – samman Dec 22 '19 at 3:07
  • \$\begingroup\$ for room........ \$\endgroup\$ – samman Dec 22 '19 at 3:08
  • \$\begingroup\$ Forgot to address the imports. I specify the imports because I plan to add entire different sections doing different things (that's why I have all those extra inputs at the begining). They might use different imports, so I specify it per application. \$\endgroup\$ – samman Dec 22 '19 at 3:30
  • 5
    \$\begingroup\$ @samman For the last point, it would probably be best to have each "part" as a separate module/file then. If each part is so different that it requires a whole different set of dependencies, they're probably different enough to warrant complete separation. The typical, overly simplified organization is each major aspect of the program is a discreet module, then you have a central "main" module that selectively uses each part. The main in this case would be the part that takes input and dispatches depending on the input given. \$\endgroup\$ – Carcigenicate Dec 22 '19 at 3:58
  • 1
    \$\begingroup\$ @samman Yes, that's what I'm saying. I would still have all imports at the top of each file though. Each file ideally shouldn't just be code that runs when it's imported. That makes it difficult to control when code runs. Each file should be made up of functions that execute the desired behavior, then you just call the relevant function when you want that code to run. Having everything as a top-level script is fine when messing around and learning, but it becomes increasingly problematic as code gets more complicated, and you start using tools like a REPL. \$\endgroup\$ – Carcigenicate Dec 22 '19 at 21:24
8
\$\begingroup\$

I'm going to put this code into an editor, "proofread" it from top to bottom, give notes as I go, and then paste the final result to show the effect of the edits.

  1. Code editors don't usually wrap lines since linebreaks mean things in code. Consequently, most coding style guidelines suggest limiting your column width so the reader doesn't need to scroll horizontally; I'm going to break up those lines as I encounter them.

  2. if A == ('Slow').lower(): is either a bug or needless. I think you want if A.lower() == 'slow' to do a case-insensitive comparison -- but given that this is the only thing you use this value for, I'm going to remove A completely.

  3. Overcome indentation! An easy way of doing this in this script is to invert your if checks and break early so you can get the main flow of control back to the left side of the page. After I applied this to all those input statements, almost the entire script ends up unindented, which makes it a lot easier to read the loops that are left.

  4. I'm going to say this as diplomatically as possible: these unindented comments in the middle of deeply indented code are literally a war crime against my eyeballs. Making the reader jump from right to left and back as they're reading the code is very unkind.

  5. I'm gonna use NotImplementedError as a way to break the control flow when we hit something that's not implemented. It's basically the error that's built into Python as a way to say "I didn't write this part yet" so using it is a very clear way to communicate that situation even if the reader can't understand your error message, and raising an exception will make Python just stop what it's doing (which is what we want here).

  6. Unless you have a really good reason otherwise, put your imports at the top of the script. It makes it easy to see in one place what the script's dependencies are, which might be important to someone else trying to use it, or to you as you do more development on it.

  7. I'm not going to try to fix this, but I think you probably have a bug in your next_path code -- you specify a fixed Windows path as the root (this of course isn't going to work the instant you try to run this somewhere else -- contrary to your stated intent of making things easy for the user, this is bound to be extremely frustrating once someone's in that situation), and then you prompt the user for a path with forward slashes. Everything is being set up for failure. I think it'd be better to just prompt the user for an absolute path, or to use the current directory as the root path.

  8. Using capitalization for variable names is kinda weird; the standard convention is for individual variables (instances) to be lowercase, and to use capitalization for class names (types). That way your eyeball can quickly pick out which is which, just like how in written English we capitalize proper nouns but not common nouns to be able to easily distinguish them.

  9. titration_file_input.lower() not in ['done'] is needless syntactic sugar for titation_file_input.lower() != 'done'.

  10. I'm gonna restructure this file reading loop a bit so that the code is simpler and the user gets faster feedback if they typo a filename.

  11. 'File' + ' ' should be written as 'File '.

  12. Adding linebreaks to these comment-offset blocks so that the comments are grouped with the code they describe, like paragraphs. Again, this is all about making it easy for human eyeballs to parse the script.

  13. Use spaces consistently! The first part of the script uses normal spacing but it's like by the end you were getting stressed out about running out of room so everything is scrunched together. :) I'm going to add spacing and also linebreaks where it seems helpful to figure out how all the args to a complicated line are organized.

  14. Giving these magic data processing formulas names would be good. Giving the numeric variables names would also be good. The short variable names like J, M etc are fine if they're local to the context of a standard formula that has a descriptive name, but just having magic variables in a magic formula makes it impossible for anyone else to know what this part of the script is doing.

  15. Why do you take your two descriptive variables, put them into a list called input_data, and then only ever address them as the individual values via that list? It's extra code that only makes the logic harder to understand. I'm just swapping that back out.

  16. A standard convention for a "placeholder" variable that is needed for the signature of a passed function but that you're not going to actually use is _ (or you can prefix each variable with _ if you have more than one).

  17. Moving the definitions of your A, B, C variables into the one function where they're used. That way the reader can see that they're only intermediate values for that one function and have no lasting meaning (which would be frustrating to try to determine given how short the names are). In general, you want to keep the scope of values as short as possible so that the reader (and the compiler/interpreter for that matter) can see when it's safe to forget about them.

Done! Here's what I've got in my editor after making all those changes (hopefully I didn't add any fat-fingered typos in there):

import os
import pandas as pd
import numpy as np
import matplotlib.pyplot as plt
from scipy.optimize import curve_fit

# I plan on adding other scripts later to analyze different data sets. 
# Currently I have written the script for one. But this is designed to 
# be a general use program, so I want to get it out there now, but add 
# features to it later. Hence all the initial inputs.
if input(
    'Slow or Fast Exchange? (Type in "Slow" or "Fast")\n'
).lower() != 'slow':
    raise NotImplementedError('Fast Exchange is not implemented yet.')
if input(
    'Will you be using both the free and bound state peaks? (Type in "Yes" or "No")\n'
).lower() == "yes":
    raise NotImplementedError('Slow Exchange analysis using both states is not developed yet')
if input(
    'Will you be using the free or bound state peaks? (Type in "Free" or "Bound")\n'
).lower() != 'free':
    raise NotImplementedError('Slow exchange analysis for the bound peak is not developed yet')

print ('Loading up Slow Exchange Script')
# This is where the user inputs where the files they will be calling are stored. 
# To make this more general user friendly, I automatically fill the drive and user portion.
# TODO: fix this
main_path=(r'C:\Users\Sams PC')
next_path=input('Please indicate the folder the files are. E.g. /Desktop/Peaklists \n')
full_path=main_path+next_path
os.chdir(full_path)

#The data that will be analyzed is split into 3 sections. One is the experimental data itself 
# (titration_files), the settings they used to obtain the experimental data (concentrations), 
# and a normalization factor (Dilution)
data_table = []
peak_height = []
peak_names=[]
dilutions=input('Enter name of Dilution file.\n')
concentrations=input('Enter name of Concentration file.\n')
# The user can input as many peaklist files as they desire. 
print('Enter name of peaklist files. When finished, type done and enter to stop.')
while True:
    data = input()
    if data.lower() == 'done':
        break
    try:
        titration_datatable = pd.read_csv(data, sep='\s+', header=None)
        titration_datatable.columns=['Column_1', 'Column_2', 'Column_3', 'Column_4', 'Column_5']
        data_table.append(titration_datatable)
    except:
        print('File ' + data + ' not found')

# The input of titration_data comes from a specific program that has the required data stored in Column 4. 
# I am creating a second table from Column 1 to use when saving the plots and savefiles.
for titration_datatable in data_table:
    peak_height.append(
        titration_datatable.loc[:, 'Column_4']
    )
    peak_names.append(
        titration_datatable.loc[:,'Column_1']
            .drop([0], axis=0)
            .drop([1], axis=0)
    )

# The first row in the datatable automatically is labeled data, so that is removed. 
# The 2nd row also has miscellanious info. The .astype is because I had to define them as integers, 
# otherwise I would get errors at the division step below. 
concatenated_titration_datatable = (pd.concat(peak_height, axis=1)
    .drop([0],axis=0)
    .drop([1],axis=0)
    .astype(int)
)
dilutions = pd.read_csv('Dilution.txt', sep='\s+', header=None)

# The data may sometimes have negative values, which can be considered the same as zero in this case.
combined = concatenated_titration_datatable.clip(lower=0)

# The data now needs to be normalized to account for dilution 
# (this is introduced in the experimental setup);
# the values are defined by a txt file the user uploads.
normalized = (combined / dilutions.values)

# It appears easier to do data modifications with a numpy matrix rather than with pandas, 
# so I converted this to numpy. I also had to do it for the function below as well.
M = pd.dataFrame.to_numpy(normalized)

# The below function is part of data processing. It's an equation for analyzing the data.
titration_data = (M[:, :1] - M) / ((M[:, :1] - M) + M)

concentrations = np.loadtxt('concentrations.txt')
protein = concentrations[:,0]
ligand = concentrations[:,1]
input_data = [protein, ligand]

# To be able to save the peak_names list above as a png for the graphs below, 
# I had to convert to a nupy array first. But this would cause errors since its a matrix, 
# thus I removed all the repeats, giving me a 1D array.
peak_names_ar=np.array(peak_names)
peak_names_array=np.unique(peak_names_ar)

# For each iteration of the function, I want to save the output of the fit (kD), 
# the standard deviation of that fit, and R2 (goodness of fit). 
# I'm also saving each plot, and using the above peak_names file to do so.
def fun(_, kd):
    a = protein
    b = protein + ligand
    c = ligand
    return np.array((b + kd - np.sqrt(((b + kd)**2) - 4*a*c))/(2*a))
kD=[]
r2=[]
standard_deviation=[]
output_for_graphing=[]
for values, i in zip(titration_data, peak_names_array):
    intensity=[values]
    intensity_array=np.array(intensity)
    x = ligand
    y = intensity_array.flatten()
    popt, pcov = curve_fit(fun, x, y)
    kD.append(popt)
    fun_data = fun(x, *popt)
    output_for_graphing.append(fun_data)
    residuals = y - fun(x, popt)
    ss_res=np.sum(residuals**2)
    ss_tot=np.sum((y - np.mean(y))**2)
    r_squared = 1 - (ss_res / ss_tot)
    r2.append(r_squared)
    std = np.sqrt(np.diag(pcov))
    standard_deviation.append(std)
    plt.plot(x, y, label='data')
    plt.plot(x, fun(x, *popt), label='fitted')
    plt.xlabel('ligand Concentration')
    plt.ylabel('intensity')
    plt.title([i])
    plt.grid()
    plt.legend()
    files_to_save = str([i])+'.png'
    plt.savefig(files_to_save)
    plt.show()

# I'm saving the output of the function as well, in case someone wants to graph it for 
# themselves and doesn't like matplotlibs display.
np.savetxt('Output for graphing.txt',output_for_graphing)

# I need to flatten the arrays to be able to stack them (I get an error otherwise)
kD_array=np.array(kD).flatten()
r2_array=np.array(r2).flatten()
standard_deviation_array=np.array(standard_deviation).flatten()
dissociation_constant=np.stack((kD_array, standard_deviation_array, r2_array), axis=-1)

# I realized after the fact that I wanted to add an extra column to the above matrix. 
# But since numpy doesn't seem to have an insert command, and I didn't want to break 
# the matrix I already had, I thought it might be easier to just change it to a pandas datatable.
dissociation_constant_table = pd.dataFrame(dissociation_constant)
dissociation_constant_table.columns = ['kD', 'Standard Deviation', 'R2']
dissociation_constant_table.insert(0, 'Files', peak_names_array)
dissociation_constant_table.to_csv('dissociation_constant.txt', sep='\t', index=False)

\$\endgroup\$
  • \$\begingroup\$ First off, thank you so much! Secondly, there were only two issues. 1) for pandas, dataframe is DataFrame, you had it as dataFrame. 2) For some reason, the 3rd input doesn't work (e.g. 'Will you be using the free or bound states'). Both answers 'free' and 'bound' give the same implemented error response. Outside of that, my main comments are: 1) The reason I had the whole nesting issue is because I thought if you wanted something done under an if statement, the rest of the code had to be under it as well (e.g. so for the if input statements, I thought the code had to be inside of the \$\endgroup\$ – samman Dec 22 '19 at 21:04
  • \$\begingroup\$ if statement if it was going to work. 2) A big chunk of the function part of the script was just because I just played around with it till it worked (e.g. didn't know about the _ command). 3) I agree with poor section of main_path. I do plan to change that pathway when I move it to the main server everyone uses (so it'll be less specific, and only have the C:\Users section), but I still haven't found a way to make it so those less tech savy can work it (I'm trying to design this so someone who knows nothing about computers can use it, e.g. if they didn't know how to check the current path \$\endgroup\$ – samman Dec 22 '19 at 21:12
  • \$\begingroup\$ My main desire in designing this is to be as "black box" and approachable as possible (e.g. simply tell me where the files are and what the filenames are and the program will do the rest). This is definitely a much cleaner version, thank you! \$\endgroup\$ – samman Dec 22 '19 at 21:15
  • \$\begingroup\$ (2a) was a typo (I changed lower() to lower). Fixed that. In (1b) you're correct about how if statements work; the key is that you can structure your program so that only a small part of it happens under the if and then the rest happens in the main control flow, which is what I did here. I.e. I only used the if to trigger the NotImplementedError and then let the flow return back to the main body in the case where that didn't happen. \$\endgroup\$ – Sam Stafford Dec 22 '19 at 22:20
  • \$\begingroup\$ BTW, most of the principles applied here come straight from Seiwald's Seven Pillars of Pretty Code: eetimes.com/seven-pillars-of-pretty-code/# \$\endgroup\$ – Sam Stafford Dec 22 '19 at 22:22
3
\$\begingroup\$

In addition to the other comments, I'll add that expecting a user to enter answers to several prompts and at least three (3) file paths without making a mistake likely leads to a poor user experience.

For example, the code:

main_path=(r'C:\Users\Sams PC')
next_path=input('Please indicate the folder the files are. E.g. /Desktop/Peaklists \n')
full_path=main_path+next_path
os.chdir(full_path)

will likely throw an exception if the user doesn't enter a '/' at the begining of the next_path.

Code a function that asks for a filename or path and then verifies that the file exists (or == 'done' or something). If it doesn't then prompt the user to reenter the filename.

Also, consider using pathlib as an easier way to manipulate filenames or paths in an OS agnostic way.

\$\endgroup\$
  • \$\begingroup\$ I decided against having the user specify the path. I want to incorporate this on linux as well, and setting a pre-defined path is just setting myself up for problems. I'm just going to have the user navigate to the folder their data is in first, via the terminal, and run the program in that directory. \$\endgroup\$ – samman Dec 23 '19 at 17:07

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.