Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Logging Statements and Avoid Crash #6

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add Logging Statements and Avoid Crash #6

wants to merge 2 commits into from

Conversation

ben-e-whitney
Copy link

The first commit adds a pair of logging statements to format_alignment. I thought it was worth alerting the user if an invalid column type was passed in.

The second commit avoids a crash if the script is passed an empty file (or if skip is large enough to skip all of the rows in a file). Compare the output of

touch empty.csv
python3 tably.py empty.csv

before and after the commit. One possible solution (the route I took) is to initialize num_columns to zero and then modify format_alignment to avoid an empty return value (which would cause a TeX error) when length is zero. If you'd like to handle it in another way you can skip this commit.

@narimiran
Copy link
Owner

Thank you for your contribution!

I've started by looking at your second commit. You are right saying that currently the script has an ugly crash when there is an empty table (either empty file or too large skip).
Thanks for bringing my attention to this!

If you'd like to handle it in another way you can skip this commit.

I think I found another, simpler, way. I'm gonna push it soon, take a look when it is here.

The first commit adds a pair of logging statements to format_alignment. I thought it was worth alerting the user if an invalid column type was passed in.

Logging idea seems interesting, I'll think about it. See if you would maybe like to add logging also in places where there is currently printing to console (and maybe in other places too), and make another pull request.

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.

2 participants