3
\$\begingroup\$

This is my first Python script and I would like to hear the opinion of more experienced users, so I can do it right from the beginning instead of trying to correct my mistakes after months (years?) of coding.

#!/usr/bin/env python

import argparse
import csv
import json
import os
import sys
from pprint import pprint

__author__  = 'RASG'
__version__ = '2018.02.15.1843'

# ------------------------------------------------------------------------------
# Argumentos
# ------------------------------------------------------------------------------

arg_parser = argparse.ArgumentParser(
    description     = 'csv <-> json converter',
    epilog          = 'Output files: /tmp/rag-*',
    formatter_class = lambda prog: argparse.RawTextHelpFormatter(prog, max_help_position=999)
)

arg_parser.add_argument('-v', '--version', action='version', version=__version__)

argslist = [
    ('-f',  '--file',   '(default: stdin) Input file',             dict(type=argparse.FileType('r'), default=sys.stdin)),
    ('-ph', '--header', '(default: %(default)s) Print csv header', dict(action='store_true')),
]

for argshort, arglong, desc, options in argslist: arg_parser.add_argument(argshort, arglong, help=desc, **options)

args = arg_parser.parse_args()

# ------------------------------------------------------------------------------
# Funcoes
# ------------------------------------------------------------------------------

def get_file_ext(f):

    file_name, file_ext = os.path.splitext(f.name)
    return file_ext.lower()


def csv_to_json(csv_file):

    csv_reader  = csv.DictReader(csv_file)
    output_file = open('/tmp/rag-parsed.json', 'wb')

    data = []
    for row in csv_reader: data.append(row)
    json_data = json.dumps(data, indent=4, sort_keys=True)
    output_file.write(json_data + '\n')


def json_to_csv(json_file):

    json_reader = json.loads(json_file.read())
    output_file = open('/tmp/rag-parsed.csv', 'wb')

    campos     = json_reader[0].keys()
    csv_writer = csv.DictWriter(output_file, fieldnames=campos, extrasaction='ignore', lineterminator='\n')

    if args.header: csv_writer.writeheader()

    for j in json_reader: csv_writer.writerow(j)


# ------------------------------------------------------------------------------
# Executar
# ------------------------------------------------------------------------------

for ext in [get_file_ext(args.file)]:

    if ext == '.csv':
        csv_to_json(args.file)
        break

    if ext == '.json':
        json_to_csv(args.file)
        break

    sys.exit('File type not allowed')
\$\endgroup\$
4
\$\begingroup\$

Before any criticism, great job. This is mature-looking code.

Alright, now some criticism ;)

  • Single-line if and for loops are not good style in Python: always break the body out into its own indented block, even if it's only a single line; for example:

    for argshort, arglong, desc, options in argslist:
        arg_parser.add_argument(argshort, arglong, help=desc, **options)
    
  • Add docstrings to your functions, to document what they do and their inputs/outputs (I prefer the Google format, but you're free to choose whatever works best for you):

    def get_file_ext(f):
        """Retrieves the file extension for a given file path
    
        Args:
            f (file): The filepath to get the extension of
    
        Returns:
            str: The lower-case extension of that file path
        """
        file_name, file_ext = os.path.splitext(f.name)
        return file_ext.lower()
    
  • There are a couple more built-ins you could use. For example, get_file_ext(f) could be replaced by Path(f.name).suffix (using pathlib, if you're using an up-to-date Python)

  • Unless there's some other good reason, use json.dump(open(...), data, ...) and json.load(open(...)) rather than reading/writing the file yourself, like json.loads(open(...).read()). This way, you never need to store the JSON text in memory, it can get saved/read to/from disk lazily by the parser. It's also just cleaner. (Also, you don't need 'wb' mode, just 'w', since JSON is text, not an arbitrary byte stream.)

  • When you do want to manually open a file, it's better practice to use it as a context manager, which will automatically close the file at the proper time:

    with open(...) as output_file:
        output_file.write(...)
    
  • Wrap the body of your script in a __main__ block:

    if __name__ == '__main__':
        for ext in [...]: ...
    

    or

    def main():
        for ext in [...]: ...
    
    if __name__ == '__main__':
        main()
    

That's more the style that's popular and standard in the Python community. You're clearly familiar with good coding, though, and it shows in your style. Good job, and welcome to Python!

\$\endgroup\$
  • \$\begingroup\$ thank you for the comments, especially about __main__. i do intend to import the module in a bigger script \$\endgroup\$ – RASG Feb 18 at 13:11

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.