1
\$\begingroup\$

The following code convert csv to ascii table.

#!/usr/bin/env python3

import sys
import argparse
import logging
import prettytable
import csv

parser = argparse.ArgumentParser()
parser.add_argument("infile", nargs="?", type=argparse.FileType("r"), default=sys.stdin)
parser.add_argument(
    '-d',
    '--debug',
    help="Print lots of debugging statements",
    action="store_const",
    dest="loglevel",
    const=logging.DEBUG,
    default=logging.WARNING,
)
parser.add_argument(
    '-v',
    '--verbose',
    help="Be verbose",
    action="store_const",
    dest="loglevel",
    const=logging.INFO,
)
args = parser.parse_args()

# taking input from stdin which is empty, so there is neither a stdin nor a file as argument
if sys.stdin.isatty() and args.infile.name == "<stdin>":
    sys.exit("Please give some input")

logging.basicConfig(level=args.loglevel)

# # Business Logic Here

table = None

content = csv.reader(args.infile, delimiter=',', quotechar='"')

for row in content:
    if table is None:
        table = prettytable.PrettyTable(row)
    else:
        table.add_row(row)

print(table)

Please let me know how I can make this code better.

\$\endgroup\$
3
  • \$\begingroup\$ You don't need a csv to print an ASCII table, Python has a builtin function that returns the character located the Unicode codepoint for any inputted integer in the range [0, 1114111]: chr. ASCII is codepoints [0, 255], in Python: range(256). Use a list comprehension to get the characters: [chr(i) for i in range(256)]. If you want a square: [[chr(16*i+j) for j in range(16)] for i in range(16)] \$\endgroup\$ Feb 12, 2022 at 13:17
  • 1
    \$\begingroup\$ @XeнεiΞэnвϵς This sounds interesting. can you please elaborate in the answer section. I could not completely understand what you are saying. \$\endgroup\$ Feb 13, 2022 at 12:43
  • \$\begingroup\$ @XeнεiΞэnвϵς, I think you have misunderstood what the code does (admittedly the description is very brief). It seems to be about formatting an existing table using ASCII formatting (-, | and +), rather than making a table of the ASCII characters. \$\endgroup\$ Feb 14, 2022 at 13:18

2 Answers 2

3
\$\begingroup\$

I don't understand the point of this test:

if sys.stdin.isatty() and args.infile.name == "<stdin>":
    sys.exit("Please give some input")

We want to disallow a particular file name, but only if we're connected to a tty? If you want to prompt when input is coming from stdin, then we shouldn't be testing the file name, but instead its properties. And I don't see why we should exit in this case (e.g. we might want to type directly, or copy-paste input). I suggest:

if args.infile.isatty():
    print("Please enter your data:")

We might want to add a hint such as Finish with control-D - I'll leave it as an exercise to discover the terminal's control bindings and print the correct keystroke.


We don't need to read one row at a time. prettytable can do that for us:

table = prettytable.from_csv(args.infile)

However, it requires the stream to be seekable, so if it's not, we'll need to read it into memory and pass that as a stream:

if not args.infile.seekable():
    from io import StringIO
    args.infile = StringIO(args.infile.read())

table = prettytable.from_csv(args.infile)

It's probably a good idea to print only the message from any exceptions, rather than an entire backtrace (unless the user asks for the extra detail):

try:
    if not args.infile.seekable():
        from io import StringIO
        args.infile = StringIO(args.infile.read())
    print(prettytable.from_csv(args.infile))
except Exception as e:
    logging.error(e, exc_info=(args.loglevel<=logging.DEBUG))
    exit(1)

Modified code

#!/usr/bin/env python3

import sys
import argparse
import logging
import prettytable
import csv

parser = argparse.ArgumentParser()
parser.add_argument(
    "infile", nargs="?",
    type=argparse.FileType("r"),
    default=sys.stdin
)
parser.add_argument(
    '-d', '--debug',
    help="Print lots of debugging statements",
    action="store_const", dest="loglevel", const=logging.DEBUG,
    default=logging.WARNING,
)
parser.add_argument(
    '-v', '--verbose',
    help="Be verbose",
    action="store_const", dest="loglevel", const=logging.INFO,
)
args = parser.parse_args()

logging.basicConfig(level=args.loglevel)

if args.infile.isatty():
    print("Please enter your data:")

try:
    if not args.infile.seekable():
        from io import StringIO
        args.infile = StringIO(args.infile.read())

    table = prettytable.from_csv(args.infile)
    print(table)
except Exception as e:
    logging.error(e, exc_info=(args.loglevel<=logging.INFO))
    exit(1)
\$\endgroup\$
5
  • \$\begingroup\$ There is a comment above the line which i know is sort of incomprehensible. We can take input from either file (0) or stdin(1). So there are four combinations. 00, 01, 10, 11. The lines you mentioned deal with 00. \$\endgroup\$ Feb 17, 2022 at 7:08
  • \$\begingroup\$ We take input from a File object, i.e. a stream, which could be connected to a terminal, a seekable file, a named pipe, etc. Your version fails if we attempt to read a file called <stdin> - e.g. python3 273995.py '<stdin>' refuses. On the other hand python3 273995.py /dev/tty doesn't issue the prompt where my version does. Do you have an example where your version does better than mine? \$\endgroup\$ Feb 17, 2022 at 8:44
  • \$\begingroup\$ I replaced everything bellow # # Business Logic Here with table = prettytable.from_csv(args.infile) and printed the table. It just works. \$\endgroup\$ Feb 17, 2022 at 9:00
  • \$\begingroup\$ I had meant to show the resultant modified program; I've now added it to the answer for convenience. \$\endgroup\$ Feb 17, 2022 at 10:26
  • \$\begingroup\$ Thank you very much for the code. \$\endgroup\$ Feb 18, 2022 at 0:56
3
\$\begingroup\$

A few things that I would change:

  1. Use functions - move some of the logic to functions; this change will enable you to reuse your code more easily in the future. It will also make unit testing much easier.

  2. sys.exit() will only print the error to standard error stream. It's preferable to use the logging package instead.

  3. Handle and communicating errors better - maybe the CSV is corrupted? Maybe it's empty and then you will print "None" to the user who will not understand what is the problem, so add more checks, log on error and better communicate the errors to the user.

\$\endgroup\$
2
  • \$\begingroup\$ Can you please add code to your answer. \$\endgroup\$ Feb 13, 2022 at 12:45
  • \$\begingroup\$ Note that demonstrative code isn't required in an answer. If a review includes suggested alternatives, that can sometimes make it more helpful, but it's not always appropriate. \$\endgroup\$ Feb 15, 2022 at 7:49

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.