4
\$\begingroup\$

I'm not a programmer, just a guy who needs to solve particular things using code.

I needed to create a JSON from a CSV with predefined headers. I'm downloading an attachment from JIRA (REST API), parsing the file and then mapping each value to a dictionary. The CSV has UTF-8 values.

# First I prepare my dic
    extracted_info = []
    mydic = {   'Ticket': None,
                'Dominio': None,
                'Tipo': None,
                'Ruta': None,
                'Version': None,
                'Componente': None,
                'Comentario': None
                }

    # Mapping values from CSV to Dictionary
    #
    def mappingThoseValues(d, a):
       # print i
        if d != "" :
            if a == 1 :
                mydic['Ticket'] = d
            elif a == 2:
                mydic['Dominio'] = d
            elif a == 3:
                mydic['Tipo'] = d
            elif a == 4:
                mydic['Ruta'] = d
            elif a == 5:
                mydic['Version'] = d
            elif a == 6:
                mydic['Componente'] = d
            elif a == 7:
                mydic['Comentario'] = d
                extracted_info.append(mydic.copy())    

    # Parse CSV
    #
    # If URL isn't empty
    if attachment_final_url != "" :
        with open(runbookname) as csvfile:
            spamreader = csv.reader(csvfile, delimiter='\t')
            a= 0
            for row in spamreader:
                for i in row :
                    d = i.decode('iso-8859-1').encode('utf8')
                    a = a + 1
                    mappingThoseValues(d,a)
                    if a == 7 :
                        a = 0
        csvfile.close()
    else :
        status_compara_rn_jira = error_format_tab + 'ERROR: No attachments in : ' + myTicket

I solved what I needed, but I would like to learn how to do this properly, and maybe someday become a real programmer.

The CSV has 7 columns. In order to identify which one I'm parsing, I've added the variable a which increments each time. When I reach the 7th column I reset the value to 0, until the end of the file.

The CSV file doesn't contain headers for each column.

\$\endgroup\$
  • \$\begingroup\$ Does your csv file contain a first row with the name of all the columns, or is it just rows of values? \$\endgroup\$ – Thanassis May 23 '17 at 14:18
4
\$\begingroup\$

What you have done follows a coding anti-pattern (a coding pattern that is to be avoided). It's ok, you are learning. This anti-pattern is called loop-switch sequence. The essence of this antipattern is that a clear set of steps is done with a loop and a switch statement (or a series of if and else).

In your case, the steps are not even that different to require a distinct statement each. What you essentially want is for the first value in a row to be assigned to the first key, the second value to the second key etc.

You do not need the function mappingThoseValues, nor the variable a. Here's how you can do your job in Python. First, have the names of your dictionary keys in a list.

key_names = ['Ticket', 'Dominio', 'Tipo', 'Ruta', 'Version', 'Componente', 'Comentario']

Then you can read your csv row values and fill in the dictionary in one simple loop.

extracted_info = []
with open(runbookname) as csvfile:
    spamreader = csv.reader(csvfile, delimiter='\t')
    for row in spamreader:
        d =  {}
        for key, value in zip(key_names, row):
            d[key] = value.decode('iso-8859-1').encode('utf8')
        extracted_info.append(d)

The important thing here is to understand the function zip and how we can use it in loops. There is plenty of educational material you can find online (example). A few other things to notice in the new code:

  • No need for the global variable mydic (in general, you should avoid using global variables to exchange data between methods).
  • No need to create copies of your dictionary to append them in the list, because each time in the loop we create a new dictionary (in your code you did need to copy mydic because it was a global variable)
  • No need for the call to close(), because you are using the with clause (close will be automatically called)

Update based on comments: You can make the code even more succinct and pythonic by employing a dictionary comprehension. This might seem strange/difficult at your experience stage. Don't worry, you can come back to it later. The important bit is to understand the previous points I raised, but I'll include the dictionary comprehension here for reference.

List and dictionary comprehensions is a pythonic way to replace (some) loops by making the loop expression more succinct and also faster. Here's how it would work for your code:

extracted_info = []
with open(runbookname) as csvfile:
    spamreader = csv.reader(csvfile, delimiter='\t')
    for row in spamreader:
        d =  {key: value.decode('iso-8859-1').encode('utf8') for (key, value) in zip(key_names, row) }
        extracted_info.append(d)

Worth mentioning that there is also a csv.DictReader class that can directly read a csv file into a directory. You can provide the names of the keys in one of the argument called fieldnames, or if you do not define any fieldnames it will use the first row of the file as names for the fields/keys. However, my preference is to use this in very common csv format (like excel), as I have found it somewhat cumbersome to define delimiters. You will still need to loop through every row of this reader to get the dictionaries, and you would still need to do any manipulations you want (like your decoding and encoding) as everything is stored in strings. So the regular reader works equally well, and produces equally succinct code.

Finally, let me comment on decoding and encoding Unicode characters. You need of course to decode the bytes you read from your file. When you decode them they become Unicode strings (in Python 2.x they are called "Unicode strings", in Python 3.x they are called "strings", they are the same thing). Instead of immediately encoding these unicode strings to another byte format (as you do) you could simply store these unicode strings to your dictionary. You encode to another byte format only when you need to write it to another file or make it a stream of bytes in any way. In fact, this is the recommended thing to do. Quoting from the linked page:

The most important tip is: Software should only work with Unicode strings internally, decoding the input data as soon as possible and encoding the output only at the end.

I hope this helps.

\$\endgroup\$
  • 2
    \$\begingroup\$ Thanks Thanassis I'm learning a lot with this :) What a magic function zip is. \$\endgroup\$ – Miguel Ortiz May 23 '17 at 17:26
  • 1
    \$\begingroup\$ You can also you dictionary comprehension instead of creating empty dictionary and setting values for keys. \$\endgroup\$ – user1685095 May 23 '17 at 18:49
  • 2
    \$\begingroup\$ ... and there is csv.DictReader which creates a dictionary per row out of the box. Just hand over the column names in the constructor: reader = csv.DictReader(csvfile, fieldnames=key_names) \$\endgroup\$ – lenz May 23 '17 at 22:12
  • 1
    \$\begingroup\$ Thanks Thanassis for the links provided. I'd like to add that the zip function is clearly explained here: docs.python.org/2/library/functions.html \$\endgroup\$ – Miguel Ortiz May 24 '17 at 14:53

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.