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

use of open() results or filehandles #9659

Closed
iwelch opened this issue Jan 7, 2015 · 9 comments · Fixed by #12739 or #12807
Closed

use of open() results or filehandles #9659

iwelch opened this issue Jan 7, 2015 · 9 comments · Fixed by #12739 or #12807
Assignees
Labels
io Involving the I/O subsystem: libuv, read, write, etc.
Milestone

Comments

@iwelch
Copy link

iwelch commented Jan 7, 2015

[novice warning]

per the discussion in https://groups.google.com/forum/?utm_source=digest&utm_medium=email#!searchin/julia-users/reading$20compressed$20csv/julia-users/1C97f24SmKo/2K5c2OdAV94J and #6948

I would suggest that all functions that work with a part of the tuple returned by open() also work with the full tuple. in particular, I would like to see familiar language constructs like the following to "just work":

close(open(...))
fs=open(`cat file.csv`); readcsv(fs); close(fs);

having to index fs is neither intuitive nor necessary.

regards,

/iaw

@elextr
Copy link

elextr commented Jan 7, 2015

A better way of doing the open(), do something, close() pattern is to use the do syntax:

open(`gzcat file.gz`, "r") do f
   readcsv(f) # or whatever do something is
end

Then if the do something throws the file still gets closed politely.

@kmsquire
Copy link
Member

kmsquire commented Jan 7, 2015

@elextr, while that construct is useful, it isn't always appilcable, and either way, the fact that open(cmd) returns a tuple seems (to me) to be somewhat inconsistent and confusing.

Ivo, thanks for opening up this issue!

Ref: #6948

@elextr
Copy link

elextr commented Jan 7, 2015

@kmsquire wasn't attempting to say don't improve it, just that for the simple use-case there is a safer way of doing the pattern (it hadn't been mentioned on the thread last I looked).

As was hinted in the thread, why not define the stream like reads and writes for the process object, it carries three asyncstreams with it.

@JeffBezanson
Copy link
Member

This is a fair point. It would be good for open(cmd) to just return a stream, with a separate function or option for getting the process object, which you don't always need.

@vtjnash
Copy link
Member

vtjnash commented Jan 13, 2015

to just return a stream, with a separate function or option for getting the process object, which you don't always need.

on the contrary, i think it should perhaps return only a process object, but that a process object should inherit from stream and redirect reads and writes to the appropriate stdin/stdout stream.

@nalimilan
Copy link
Member

on the contrary, i think it should perhaps return only a process object, but that a process object should inherit from stream and redirect reads and writes to the appropriate stdin/stdout stream.

FWIW, that was how I envisioned it too.

@ihnorton ihnorton added io Involving the I/O subsystem: libuv, read, write, etc. needs decision A decision on this change is needed labels Jan 30, 2015
@tkelman
Copy link
Contributor

tkelman commented Feb 28, 2016

This was actually not closed by #12739, but it would be closed by #12807, so reopening

@tkelman tkelman reopened this Feb 28, 2016
@StefanKarpinski StefanKarpinski removed the needs decision A decision on this change is needed label Sep 13, 2016
@StefanKarpinski StefanKarpinski added this to the 0.6.0 milestone Sep 13, 2016
@StefanKarpinski
Copy link
Member

It seems to be decided that we return a process object that you can read and write to.

@tkelman
Copy link
Contributor

tkelman commented Jan 19, 2017

quoting Jameson - "not the top of the priority list," so isn't making 0.6

vtjnash added a commit that referenced this issue Apr 24, 2017
…lue)

This reverts commit 8ffdfc2,
and fixes it a bit too :)

fix #9659
vtjnash added a commit that referenced this issue May 2, 2017
…lue)

This reverts commit 8ffdfc2,
and fixes it a bit too :)

fix #9659
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
io Involving the I/O subsystem: libuv, read, write, etc.
Projects
None yet
9 participants