1
\$\begingroup\$

I have been using Python for only a few days, so I am trying to learn about some best practices. An explanation of what this code is supposed to do is at the bottom of this post. It is an exercise to rewrite a script I made in VBA a while ago and I am doing it to learn Python.

Basically the strings of seemingly random letters belong in one long string, but the output from its original source comes in blocks like you see in the text file. I am trying to string the letter sequences together and keep them with their appropriate organism.

http://i.imgur.com/5zilZqV.jpg

My end game is to move this information into Excel using Python, but I am not at that stage yet.

Here are the first two stanzas of the SampleSequences.txt to be parsed:

gi|499323459|WP_011013951.1|C-glutam         MEDAQPEQITATEELAGLLENPTNLEGKLADAEEEIILEGEDTQASLNWSVIVPALVIVL
gi|459384007|AGG66298.1|C-call               meDAHSEQVTATEELAGLLKNPDXVGEQLADFEEEIVFESESETASLDWTVIIPALAVVL
gi|814963457|AKE41823.1|C-kuts               MTDqsFSETTATEQLATMLAGREVIEEHAAQPEEEIIYEGEDEDAKIDFKVAIPAVAIVL
gi|334696457|AEG81254.1|C-ulcer              meSSSAGATSATAQLAAMLSGEDKISAASNLPEEKITFEGEDSEARIDWKVTIPAFVIVL
gi|504084851|WP_014318845.1|C-dip            mtTSSAPVVSATAELAAMIEGEGLIAEATAAPEKEITFEGEDDNAGVDWKVTAPAMVIVL
gi|828161585|WP_047252615.1|C-testu          MTneQAEPSTATEQLASMMSDAELIGGHLDETSDAVDRHEEDDKPSFDWAVILPTAGVIV
gi|644967131|WP_025387886.1|C-casei          mnDTYETNESATGQLASMLDTDFNPSAXXHPVEEEIELAADQKDAKIDWTITGIAGVLVA
gi|491466010|WP_005323773.1|C-pseudogen      mvDEQLDAQSATGQLQDMIHSDGFHPEXXRYDDPEVEIAADRDNMPIDWSIVGIAGVLIA
gi|917407105|P_052013817.1|P-pseudodip       mvREDLDELSATRQIAYMLEHEDDXXVAVSSVEDSVELASETEDSSIDWAVVAPAGLLVL
gi|516649031|WP_018018020.1|C-capito         MAEStpGEQTATSELASLLHADRttLANYSGAETPEPLAAEDENAPVDWAIIIPLAGIIV
                                             *        :** ::  ::             .       :     .:: :      :: 

gi|499323459|WP_011013951.1|C-glutam         ATVVWGIGFKDSFTNFASSALSAVVDNLGWAFILFGTVFVFFIVVIAASKFGTIRLGRID
gi|459384007|AGG66298.1|C-call               ATVVWGIGFNESFSKVAGSALGFVVNNLGWAFVLFGTVFVFFAIAIAASKFGQIRLGRID
gi|814963457|AKE41823.1|C-kuts               GVVIWGLLAGDSFSTFASTGLSWVVTNLGWAFILFSSVFLFFAIALAVSNFGTIKLGRTD
gi|334696457|AEG81254.1|C-ulcer              AVVLWGIFGTSSFSTFASDALSFVVSDFGWAFIFFTTVFLVFAIVIAVSNFGTIKLGRND
gi|504084851|WP_014318845.1|C-dip            AVVAWGILGTTSFASFAASSLGYIVDSFGWAFIFFTTVFLIFAIAIALSNFGTIKLGRND
gi|828161585|WP_047252615.1|C-testu          AVVLWGLLSPDSFANFASSALSLVVNNLGWAFVLFGTVFVAFMIIIAASKFGQIKLGASD
gi|644967131|WP_025387886.1|C-casei          AVVVWGLSAPDTFADVSAKALNFVVNNFGWAFILFTTVFVVFVIGIAASKFGSIKLGAIN
gi|491466010|WP_005323773.1|C-pseudogen      AIVVWGLAAPDNFANFSSSALTWVVDNFGWAYILFGTVFVVFVVFIAASKFGSIKLGHIN
gi|917407105|P_052013817.1|P-pseudodip       ATVIWGLAFTDSFTAFASTAFGWIIDNFGWAFMLFTTVFVVFVIAIAASKFGAIKLGSIN
gi|516649031|WP_018018020.1|C-capito         ALVTWGLAGAGSFSSFADTSFSWVIDNLGWAFVLFGTVFVFFVLFIAFSSFGSIRLGGPD
                                             . * **:    .*: .:  .:  :: .:***:::* :**: * : :* *.** *:**  :

My code:

main_seq_two = open("SampleSequences.txt","r")
org_dict = {}
myOffset = 0

with main_seq_two as m:
    content = m.read().splitlines()

for item in content:

    myStr = str(item) # Turn entire line into a string
    myStrLast60 = str(item)[-60:] # Last 60 characters of the line string
    myStrLen = len(myStr)
    myNameStripped = myStr[:myStrLen - 60].strip()

    if "gi|" in myNameStripped: # Checks to see if line is normal contents
        if " " in myStrLast60: # To delete empty spaces in an AA string
            myStrSymbols = myStrLast60 # To store myStrSymbols and delete extra space in myStrLast60
            myStrLast60 = str(myStrLast60.replace(" ", '')) # Replace spaces with nothing          
            myOffset = len(myStrSymbols) - len(myStrLast60) # Stores text offset for last block
        if myNameStripped not in org_dict.keys(): # Creates dictionary key if not exist yet
            org_dict.update({myNameStripped : ''}) # Initializes dictionary key      
        temp_str = str(org_dict[myNameStripped]) # Temp string for currently built AA string
        org_dict.update({myNameStripped : temp_str + myStrLast60}) # Updates organism key by adding most recent line of AA

    if "gi|" not in myNameStripped and myStrLen > 1: # Checks to see if line is symbol contents
        if "Symbols" not in org_dict.keys(): # Creates dictionary key if not exist yet
            org_dict.update({"Symbols" : ''}) # Initializes dictionary key
        if myOffset > 0: # Checks text offset for alignment, see line 15 - 18
            myStrLast60 = myStrLast60[myOffset:60] # Snips myOffset chars from beginning of Symbols String
        temp_symbols = str(org_dict["Symbols"]) # Stores currently built Symbols String
        org_dict.update({"Symbols" : temp_symbols + myStrLast60}) # Updates symbols dictionary to finalize Symbols String


for key, value in org_dict.items():
    print(key, value)

I am posting this to get some input from the community so I can learn other ways of doing things. I have some string parsing code here which gives me the output that I am looking for. I was wondering if anyone can give me some tips on what code is bad, or where / how this could be condensed, what I've done here which is bad coding practice, etc. Even just a single command which could cut down a line or two would be great.

\$\endgroup\$
5
  • \$\begingroup\$ I can explain using a picture I suppose... Basically the strings of seemingly random letters belong in one long string, but the output from its original source comes in blocks like you see in the text file. I am trying to string the letter sequences together and keep them with their appropriate organism. i.imgur.com/5zilZqV.jpg My end game is to move this information into Excel using Python, but I am not at that stage yet. Here is what the desired outcome is: dropbox.com/s/b2j5o7hb2gxh51l/Output.txt?dl=0 \$\endgroup\$ Commented Jan 15, 2016 at 20:49
  • \$\begingroup\$ i think you need to reduce this to a minimal case in order for us to get you to help on on it. \$\endgroup\$
    – rbp
    Commented Jan 15, 2016 at 21:04
  • 1
    \$\begingroup\$ Thanks 200_success for the edit. This is my first question here and now I see the proper way to seek help. Really appreciate it! \$\endgroup\$ Commented Jan 15, 2016 at 21:09
  • \$\begingroup\$ @rbp We have the sample input, desired output, and code that claims to accomplish the task. I think that's a clear enough question for Code Review. \$\endgroup\$ Commented Jan 15, 2016 at 21:14
  • \$\begingroup\$ much better now since the edit. \$\endgroup\$
    – rbp
    Commented Jan 15, 2016 at 21:14

1 Answer 1

1
\$\begingroup\$

Overall remarks

Try to use fewer variables; it's hard to keep track of them all. You should follow a consistent naming convention; PEP 8 recommends lower_case_with_underscores for variables.

Prefixing your variable names with my… serves no purpose. Variables like item and myStr should be named more purposefully.

You don't need to call str() to convert strings into strings.

Don't comment every line — it's annoying. It doubles the text I have to read, and it increases your code maintenance burden.

Input file handling

Use the fileinput module instead of hard-coding a filename in your code. You can either specify the input file as a command-line argument or pipe it to your program.

Process the file line by line instead of reading the entire file into memory at once. It's a good habit for scalability.

File format

Are you sure that you want to split each line 60 characters from the end? Based on your sample file, it seems to make more sense to take the first 45 characters as one column, and the rest of the line as the second column. That would spare you the trouble of deleting spaces from the second column. It would also avoid accidentally treating part of the label as sequence data if the sequence is too short (shorter than in the last stanza of your SampleSequences.txt).

You want if myNameStripped.startswith('gi|') rather than if "gi|" in myNameStripped. The latter would look for "gi|" anywhere in the first column.

Instead of using "Symbols" as a special key of org_dict, I recommend making a separate variable.

Suggested solution

Simpler version:

import fileinput

org_dict = {}
symbols = ''

for line in fileinput.input():
    label, content = line[:45].rstrip(), line[45:].rstrip()
    if label.startswith('gi|'):
        org_dict[label] = org_dict.get(label, '') + content
    elif content and not label:
        symbols += content

for key, value in org_dict.items():
    print(key, value)

print(symbols)

More efficient version, because Python strings are immutable, and thus repeated string concatenation is not recommended:

import fileinput

org_dict = {}    # Keys are creature labels; values are lists of AA strings
symbols = []

for line in fileinput.input():
    label, content = line[:45].rstrip(), line[45:].rstrip()
    if label.startswith('gi|'):
        org_dict.setdefault(label, []).append(content)
    elif content and not label:
        symbols.append(content)

# Collate data into strings
org_dict = {label: ''.join(contents) for label, contents in org_dict.items()}
symbols = ''.join(symbols)

for key, value in org_dict.items():
    print(key, value)

print(symbols)
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.