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

refactored file-or-str opening. fixes #191 #208

Merged
merged 1 commit into from
Jul 15, 2016
Merged

refactored file-or-str opening. fixes #191 #208

merged 1 commit into from
Jul 15, 2016

Conversation

bmcfee
Copy link
Collaborator

@bmcfee bmcfee commented Jul 10, 2016

This PR implements #191, and is mainly lifted (and simplified) from a similar helper in JAMS.

Changes:

  • A new (private) helper function _open consolidates repeated code for handling string-or-file-like inputs in IO.
  • The new function uses contextlib to properly handle file descriptor cleanup.
  • The exception type for load failure was changed from ValueError to IOError; tests were updated accordingly

@bmcfee bmcfee added this to the 0.4 milestone Jul 10, 2016
@craffel
Copy link
Owner

craffel commented Jul 13, 2016

Awesome, thank you. I assume this is tried and true since it appeared in JAMS first. The only pedantic comment I have is that _open is not a very descriptive function name, I would call it something like open_file_or_str (or _open_file_or_str if you insist on the pseudo-private function/not supplying a typical docstring). Change that if you want, then feel free to squash and merge!

@bmcfee bmcfee merged commit df34851 into master Jul 15, 2016
@bmcfee bmcfee deleted the open-context branch July 15, 2016 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants