Combining values corresponding to the same key

Question

What the following code does is:

The key field is column0 (where sometimes there could be single key, and sometimes keys separated by comma).

The rest of the columns are also either single or comma separated. But based on the single key value from column0, the goal is to collect all the values as a set in the rest of the columns.

import sys
import csv

dict={}

def getVal(k):
  try:
    v = dict[k]
  except:
    v= None
  return v

# First read as a line and first transformation separates first column
# and stores into a table with key,value (where value is the remaining)
for line in sys.stdin:
  line = line.strip()
  row = line.split('\t')
  l = len(row)
  keyvalues = row[0]
  items = keyvalues.split(",")
  for item in items:
    key = item
    value=[]
    for i in range(1, l):
       value.append(row[i].split(","))
    if getVal(key) == None:
      dict[key] = value
    else:
      prevval = dict[key]
      cols = len(prevval)
      newvalue=[]
      newlist = []
      for j in range(0,cols):
        newlist = prevval[j]+value[j]
        newset = set(newlist)
        newvalue.append(list(newset))
      dict[key] = newvalue


for k,v in dict.items():
   rowstr = k+"\t"
   ncols = len(v)
   for items in v:
     cols=""
     for item in items:
       cols +=item+","
     cols = cols[0:len(cols)-1]
     rowstr += cols+"\t"
   print rowstr 

Sample input

3,15,75 1635,1762   878 3425    121,122,123
15  1762    871 3475    121,125,126
3   1585,1590,5192  882,832,841 3200,3211   120,121,122,123,124

I'm getting the results correctly as expected, but I would like to know any improvements to make on the code.

Answer 1

Indentation matters a lot in Python. Please follow the PEP 8 standard, which is 4 spaces.

This code feels very procedural. Python can be much more expressive than that, if you take advantage of features like list comprehensions.

You've used a lot of variables: line, row, l, keyvalues, items, item, key, value, i, prevval, cols, newvalue, newlist, j, newset — too many for me to keep track of what each represents. You could reduce the mental load by eliminating variables for ephemeral expressions. For example, you could eliminate items by writing for item in keyvalues.split(",").

I see that you considered using the csv module, but didn't. If you did, you could have simplified

for line in sys.stdin:
  line = line.strip()
  row = line.split('\t')

… to for row in csv.reader(fileinput.input(), delimiter='\t'). I recommend fileinput instead of sys.stdin so that the user has the option of passing a filename on the command line.

The getVal(k) function can just be replaced by dict.get(k, None). It would be better to avoid choosing dict as the name of a variable, since it shadows the dict() constructor.

Suggested solution

import csv
import fileinput

data = {}

for row in csv.reader(fileinput.input(), delimiter='\t'):
    values = [set(col.split(',')) for col in row]
    for key in values.pop(0):   # Numbers in column 0
        data[key] = [
            new_col.union(old_col)
            for new_col, old_col in zip(values, data.get(key, values))
        ]

for key, values in data.items():
    print '\t'.join([key] + [','.join(col) for col in values])

Answer 2

Dictionary usage

Your getVal function is the get method of standard dictionaries. Use dict.get(key) instead of getVal(key). However, 3 things that are wrong with it:

• It uses the global variable dict to retrieve values from, you should have parametrize it with the dictionary instead:

  def getVal(dict_, key):
      try:
          v = dict_[key]
      except:
          v = None
      return v
  

• It uses dict as a variable name, shadowing the builtin function.

• It uses a raw except, catching the expected KeyError but potentially catching more serious issues. Always specify the kind of exception you are expecting.

Now for getVal usage, you set a value into dict[key] if getVal returned None otherwise you process further. In such case, you will set the value because the flow got into the except in your getVal function, meaning key is not in dict. It's quite simple to test in python:

if key not in dict:
    dict[key] = value
else:
    prevval = …

No need for getVal anymore…