Skip to content

commiting csvhead file#1

Open
MariamSh wants to merge 6 commits into
masterfrom
course_project
Open

commiting csvhead file#1
MariamSh wants to merge 6 commits into
masterfrom
course_project

Conversation

@MariamSh
Copy link
Copy Markdown
Collaborator

@MariamSh MariamSh commented May 9, 2017

No description provided.

Copy link
Copy Markdown
Collaborator

@onponomarev onponomarev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pylint would have detected the unused function:)

So, don't forget to use pylint.

Comment thread csvhead.py Outdated
from argparse import ArgumentParser
import sys

DESCRIPTION = 'csvpp - Print header and first lines of input.'
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the description

Comment thread csvhead.py Outdated
DESCRIPTION = 'csvpp - Print header and first lines of input.'
EXAMPLES = 'example: cat file.csv | csvhead -n 100'

def print_row(row, column_widths, output_stream):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't use this function.

Comment thread csvhead.py Outdated
Reads line per line and writes it in output_stream.
"""
args = parse_args()
print(args)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this print

Comment thread csvhead.py Outdated
input_stream = open(args.file, 'r') if args.file else sys.stdin
output_stream = open(args.output_file, 'r') if args.output_file else sys.stdout

lines = input_stream.readlines()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line loads the whole file into memory. Do you really want it? And what if the file size is 10GB? I guess that it still should be possible to implement low-overhead csvhead.

@MariamSh
Copy link
Copy Markdown
Collaborator Author

Thank you for replies, I fixed all the notes. Please review them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants