6
\$\begingroup\$

I am new to programming and entirely self-taught. Below is a program I wrote to retrieve text content via an API and scan the text for important words. If the word is found URI is returned. The API returns data in JSON. The important words and URIs are linked via a dictionary. It works. I'm at a point in my programming education where I need to start focusing on writing clean code. I have a book about it, but there's something about peer review that is so much more helpful. Here is the code with commenting. Any ideas on how I can make this cleaner?

#!/usr/bin/env python
# coding: utf-8
from __future__ import division
import urllib, json

#bio_dict is the dictionary used to assign DBpedia URIs. When a specific key is
#found in the text, the value URI is returned. Any dictionary can be used here.

bio_dict = eval(open('biodict.txt').read())

in_file_name = "text_object_id1.txt"
in_file = open(in_file_name, 'r')
out_file_name = "all_uri.txt"
out_file = open(out_file_name, 'w')

#At this point, you have imported all the modules you need, read the dictionary
#you will use to the assign the URIs and opened all the files you will be using. The
#InFile should be a list of DataObject IDs. The OutFile will hold your results. 

def data_object_url(line):
    return 'http://eol.org/api/data_objects/1.0/' + line.strip() + '.json'

#data_object_url is a function that will create the URL needed to query the API
#for each data object ID in the list

for line in in_file:                                        
    results = urllib.urlopen(data_object_url(line)).read()
    print line
    print len(results)
    data = json.loads(results)
    descriptions = data ['dataObjects'][0]['description'].lower()   
    for k, v in bio_dict.iteritems(): 
        if k in descriptions:
            out_file.write(','.join([line.strip(),k,v + '\n']))                                                                                                                                                                     
in_file.close()                                                                                         
out_file.close()

#This code goes over each line in the in_file (a list of text object IDs), retrieves
#them from the API in json format, pulls out the description text and then iterates over
#each key in the dictionary. If the key is found the value is returned and the system
#moves on to the next key. If the key is not found, the system returns nothing and moves
#on to the next key. Line 34 controls the format of the output. Lines 28 and 29 serve to
#show progress as the program runs.
\$\endgroup\$

2 Answers 2

6
\$\begingroup\$

I have a few pointers, a lot of them coming from Python's offical style guide:

  1. ALWAYS use with syntax when dealing with files:

    with open('some_file.txt', 'r') as file:
        # Do stuff
    
    # file will be closed by now
    foo = 'Hello World!'
    

    This syntax prevents any errant bugs like forgetting to close a file, which you do here:

    bio_dict = eval(open('biodict.txt').read())
    
  2. Use format to create strings with variables. This is the easiest way to create strings without having to use ugly string concatenation:

    # This functions...
    def data_object_url(line):
        return 'http://eol.org/api/data_objects/1.0/' + line.strip() + '.json'
    
    # But this is better, more Pythonic
    def data_object_url(line):
        return 'http://eol.org/api/data_objects/1.0/{}.json'.format(line.strip())
    
  3. Different module imports should not go on the same line:

    import urllib, json
    

    They should have their own line. If you are importing submodules, then you can have multiple imports on the same line:

    from my_module import some_func, other_func
    
  4. Pay attention to function and variable names; make them descriptive.

    Function names should start with a verb and describe what they do: instead of data_object_url use something like get_object_url.

    Also, make variable names descriptive; erring on the side of being too wordy than not wordy enough: instead of for k, v in bio_dict.iteritems() use for key, value in bio_dict.iteritems().

  5. When writing comments, only say what needs to be explained. Comments should not explain the obvious but instead should explain the complex or the reasons behind the code. A lot of the comments you have would actually be more fitting as docstrings.

  6. Look into list comprehensions. In my code below I was able to take your for loop into a single statement. This also made writing to the output file much more Pythonic.


Here is what your code looks like with my suggestions implemented:

#!/usr/bin/env python
# coding: utf-8

import urllib
import json

with open('biodict.txt', 'r') as file:
    bio_dict = eval(file.read())

in_file_name = "text_object_id1.txt"
out_file_name = "all_uri.txt"

def get_object_url(line):
    return 'http://eol.org/api/data_objects/1.0/{}.json'.format(line)

out_file_text = []
with open(in_file_name, 'r') as file:
    for line in in_file:   
        line = line.strip()                                     
        results = urllib.urlopen(get_object_url(line)).read()
        data = json.loads(results)

        descriptions = data['dataObjects'][0]['description'].lower()   
        out_file_text += [','.join(line, key, bio_dict[key])
                             for key in bio_dict if key in descriptions]

with open(out_file_name, 'w') as file:
    file.write('\n'.join(out_file_text))
\$\endgroup\$
0
3
\$\begingroup\$

In addition to DarinDouglass' excellent answer, I recommend sticking with a single loop over both files, rather than saving an intermediate list. That is good practice because it makes your code only need memory for one line at a time and allows you to process files of several gigabytes.

You can open two files in one with statement:

with open(in_file_name, 'r') as in_file, open(out_file_name, 'w') as out_file:
\$\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.