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

Simplify write_to and context manager usage. #118

Merged
merged 4 commits into from
Apr 30, 2015

Conversation

mdboom
Copy link
Contributor

@mdboom mdboom commented Apr 29, 2015

This is an alternative to #116. Fix #107, fix #109.

This is an attempt to fix up context manager usage at the expense of:

  • the streaming API which is now a little less convenient (since it relies on having a raw file handle). But that's advanced usage anyway
  • It's no longer possible to write_to and then update -- one must open and then update.

@astrofrog: Your thoughts?

@astrofrog
Copy link
Contributor

@mdboom - this looks much more intuitive to me, thanks! I agree that the streaming API is more advanced so it's less important for that to be convenient.

@mdboom
Copy link
Contributor Author

mdboom commented Apr 29, 2015

@nden: This is going to be a bit of a breaking API change. Basically read is now open, and write_to is no longer a context manager (it automatically closes the file and should not be used with with). Let me know if you have any projects that need to be brought over.

@mdboom mdboom force-pushed the context-manager-simplification branch from b56f781 to 617ec90 Compare April 29, 2015 18:13
mdboom added a commit that referenced this pull request Apr 30, 2015
Simplify write_to and context manager usage.
@mdboom mdboom merged commit 9f74b4c into asdf-format:master Apr 30, 2015
@embray
Copy link
Contributor

embray commented Apr 30, 2015

I guess this is already merged, but I think I like this better too. I've seen other recent cases where people were confused (in a non-ASDF context, I think) about using read/write on objects that are not stream-like objects and don't have the same semantics as, say, file.read and file.write.

mdboom pushed a commit that referenced this pull request May 5, 2015
Update note regarding layout of docs directory (see #118).
@embray
Copy link
Contributor

embray commented May 5, 2015

I feel like it might be desirable to salvage something like the streaming API in the future, only even better--allow writing to a file object, but with protections in place to ensure that it only writes to the correct places, etc. But that can be for another time. In the meantime I also find this more intuitive 👍

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.

Inconsistent naming of read/write_to? Context manager clarification
3 participants