10
\$\begingroup\$

In order to answer a given question on Ask Ubuntu I came up with two scripts for doing the following task:

Compare two files for an occurrence of column no. 6 value in file1 occurs in file2. On finding data output the line and if not do some other task.

Given example of data in file1:

352075|6505286781|6505286781|165|A|2.6.37-3.2|25353gb1FAa8
172238|8136090512|8136090512|1|A|2.6.37-3.2|255411429E02
105767|1783707658|1783707658|82|A|2.6.37-3.2|375g1043DC92
352092|2214612065|2214612065|22|A|2.6.37-3.2|07k9975cad1e

Given example of data in file2:

274451|24575|24575|3872531727|23|24575|2017-11-08 11:43:21.15|2017-11-25 16:30:21.061|STB|375g1043DC92|375g1043DC92
506406|280335|280335|4516157218|22|280335|2017-11-22 15:44:54.307|2017-11-29 11:26:02.123|STB|256d9739d3cc|256d9739d3cc
367536|163226|163226|5007632889|9|163226|2017-11-15 20:37:02.034|2017-11-28 20:55:24.891|STB|25353gb1FAa8|25353gb1FAa8
374253|254874|254874|9263432532|23|254874|2017-11-16 19:17:52.827|2017-11-28 19:25:23.805|STB|37fe9739b5a0|37fe9739b5a0

In order to Answer this question I came up with two different scripts which both produce the same output:

25353gb1FAa8 not found, doing other operation
25353gb1FAa8 not found, doing other operation
25353gb1FAa8 found in ['367536', '163226', '163226', '5007632889', '9', '163226', '2017-11-15 20:37:02.034', '2017-11-28 20:55:24.891', 'STB', '25353gb1FAa8', '25353gb1FAa8']
25353gb1FAa8 not found, doing other operation
255411429E02 not found, doing other operation
255411429E02 not found, doing other operation
255411429E02 not found, doing other operation
255411429E02 not found, doing other operation
375g1043DC92 found in ['274451', '24575', '24575', '3872531727', '23', '24575', '2017-11-08 11:43:21.15', '2017-11-25 16:30:21.061', 'STB', '375g1043DC92', '375g1043DC92']
375g1043DC92 not found, doing other operation
375g1043DC92 not found, doing other operation
375g1043DC92 not found, doing other operation
07k9975cad1e not found, doing other operation
07k9975cad1e not found, doing other operation
07k9975cad1e not found, doing other operation
07k9975cad1e not found, doing other operation

Bear in mind I'm a mere hobbyist and would put myself somewhere in between beginner and novice skill level. I would love to have a pair of eyes on my resulting scripts I answered with to get some insight what could be done better if.

Here the two scripts I came up with:

  1. Using Python built-ins:

    #!/usr/bin/env python3
    
    from sys import argv
    
    
    def parse_files(name_1, name_2):
        """Opens two files and checks if pos #6 in each row of file 1 is present in a row in file2 via simple build-ins"""
        try:
            with open(file=name_1, mode='r') as file_1:
                with open(file=name_2, mode='r') as file_2:
                    data_1 = file_1.readlines()
                    data_2 = file_2.readlines()
                    for row_1 in data_1:
                        line_1 = row_1.strip('\n').split('|')
                        for row_2 in data_2:
                            line_2 = row_2.strip('\n').split('|')
                            if line_1[6] in line_2:
                                print('{} found in {}'.format(line_1[6], line_2))
                            else:
                                print('{} not found, doing other operation'.format(line_1[6]))
                    file_2.close()
                file_1.close()
        except FileNotFoundError as error:
            print('{}'.format(error))
            exit(1)
        else:
            return
    
    
    if __name__ == "__main__":
        if len(argv) <= 1:
            print('No parameters given...')
            exit(1)
        elif len(argv) == 2:
            print('Only one file was given...')
            exit(1)
        else:
            parse_files(argv[1], argv[2])
            exit(0)
    
  2. Using the csv library:

    #!/usr/bin/env python3
    
    import csv
    from sys import argv
    
    
    def parse_files(name_1, name_2):
        """Opens two files and checks if pos #6 in each row of file 1 is present in a row in file2 via csv library"""
        try:
            with open(file=name_1, mode='r') as file_1:
                with open(file=name_2, mode='r') as file_2:
                    data_1 = file_1.readlines()
                    data_2 = file_2.readlines()
                    for row_1 in csv.reader(data_1, delimiter='|'):
                        for row_2 in csv.reader(data_2, delimiter='|'):
                            if row_1[6] in row_2:
                                print('{} found in {}'.format(row_1[6], row_2))
                            else:
                                print('{} not found, doing other operation'.format(row_1[6]))
                    file_2.close()
                file_1.close()
        except FileNotFoundError as error:
            print('{}'.format(error))
            exit(1)
        else:
            return
    
    
    if __name__ == "__main__":
        if len(argv) <= 1:
            print('No parameters given...')
            exit(1)
        elif len(argv) == 2:
            print('Only one file was given...')
            exit(1)
        else:
            parse_files(argv[1], argv[2])
            exit(0)
    

As said above, both produce the exact same output so my questions is:

  • Is the style ok?
  • Did I do any horrible faux pas?
  • What can be done better/more efficient?
\$\endgroup\$
10
\$\begingroup\$

There are things to improve:

  • when you use with context manager, you don't need to close the file explicitly
  • you can combine two context managers into one reducing the nestedness
  • there is no need to call readlines() in order to initialize a csv.reader instance
  • we can also reduce the computational complexity of your solution - currently, you are iterating for every line in both files making the overall time complexity \$O(N * M)\$ where N and M are the number of lines in file 1 and 2 respectively. We can do better than that. What if you would make a dictionary with last column values as keys and lines as values. Then, when iterating over the rows in the first file you are going to perform lookups in the dictionary, which are going to be constant time leading to overall time complexity of \$O(N + M)\$ - classic space/time tradeoff, sacrificing some space for time

All these things applied:

with open(name_1) as file_1, open(name_2) as file_2:
    mapping = {
        row[-1]: row
        for row in csv.reader(file_2, delimiter='|')
    }

    for row in csv.reader(file_1, delimiter='|'):
        last_column = row[-1]
        if last_column in mapping:
            print(f'{last_column} found in {mapping[last_column]}')
        else:
            print(f'{last_column} not found, doing other operation')

Also note the use of f-strings for formatted strings.

\$\endgroup\$
  • \$\begingroup\$ Thats already giving me a pretty good insight on what to improve, If you don't mind I still would love to give the whole questions about 24 hours before I chose which to accept. meanwhile I'm going to read up on the leads you gave me. \$\endgroup\$ – Videonauth Dec 3 '17 at 3:24
4
\$\begingroup\$

To add to the previous answer, you are using sys.argv but I would suggest you to use argparse instead because it is more convenient.

I also would suggest you to move all what follows if __name__ == "__main__" into a function to call on need.

Let us code the 2 suggestions:

def main():
   parser = argparse.ArgumentParser(description="Searching for data from file1 in file2")
   parser.add_argument("file1", help="Path to file 1.")
   parser.add_argument("file2", help="Path to file 2.")
   args = parser.parse_args()
   if args.file1 and args.file2:
       parse_files(args.file1, args.file2)

Then you can use this function if the module is executed:

if __name__ == '__main__':
    main()

Now if you call this program as solution.py, and if the user executes it without specifying the paths to both files, this error message will be triggered:

usage: solution.py [-h] file1 file2
solution.py: error: the following arguments are required: file1, file2

If the user wants to get help on how to use this module, he has to type:

python3 solution.py --help

and this help message will be displayed:

usage: solution.py [-h] file1 file2

Searching for data from file1 in file2

positional arguments:
  file1       Path to file 1.
  file2       Path to file 2.

optional arguments:
  -h, --help  show this help message and exit
\$\endgroup\$
  • \$\begingroup\$ +1; duly noted, but wont include that into my answer on AU in fear I might confuse the OP who, I assume, as a complete beginner. Will use this for myself in future projects. \$\endgroup\$ – Videonauth Dec 3 '17 at 16:30

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.