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

fix: allow CSV read exception to propagate instead of catching it #93

Merged
merged 1 commit into from
Jul 26, 2021

Conversation

ToddFincannon
Copy link
Collaborator

@ToddFincannon ToddFincannon commented Jul 26, 2021

Removed the try/catch block as agreed.

This is an additional fix for #81 as discussed in PR #82; will be merged into the todd/81-direct-data-csv branch.

Chris edit: I'm pasting the additional context from Todd's comment from #82:

I removed the try/catch block around the CSV parse. I tested it with a bad pathname and got the following unhandled exception error message:

Error: ENOENT: no such file or directory, open '/Users/toddfincannon/Projects/SDEverywhere/models/directdata/x_data.csv'

think this is clear enough that we don't need a special error message.

@chrispcampbell chrispcampbell changed the title fix: allow CSV read exception to propagate instead of catching it (#81) fix: allow CSV read exception to propagate instead of catching it Jul 26, 2021
Copy link
Contributor

@chrispcampbell chrispcampbell left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks. I'll merge this into the other 81 branch first, and then I'll merge it all together into develop.

@chrispcampbell chrispcampbell merged commit 9f9918f into todd/81-direct-data-csv Jul 26, 2021
@chrispcampbell chrispcampbell deleted the todd/81-throw branch July 26, 2021 22:11
@ToddFincannon ToddFincannon restored the todd/81-throw branch July 27, 2021 00:11
@ToddFincannon ToddFincannon deleted the todd/81-throw branch July 27, 2021 00:15
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