-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
define IOContext(io, pairs...) and remove IOContext(io; kw...) #23271
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
Conversation
706d347 to
b554bfd
Compare
|
So a bit of history:
In other words, having two concurrent APIs is kind of messy; as the prefered syle seems to use pairs, and it's faster (as of today), this confirms my impression that we should get rid of the keyword version, which can I think accomplish nothing more than the pairs version. So I added a deprecation, and I will remove the WIP. As the main authors of |
b554bfd to
830b020
Compare
| """ | ||
| IOContext(io::IO; properties...) | ||
| unwrapcontext(io::IO) = io, ImmutableDict{Symbol,Any}() | ||
| unwrapcontext(io::IOContext) = io.io, io.dict |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I introduced these two methods as they often allow to merge two methods (for IO and for IOContext) into one; moreover, they make it easy to spot what is discarded (e.g. in IOContext(unwrapcontext(io)[1], unwrapcontext(context)[2]), only the stream of io and the dict of context are kept).
|
If you feel like fixing an issue while you are at it, there is #22637 to add some more examples :) |
OK, I can do that! but will use another PR. |
|
|
StefanKarpinski
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that this is much nicer and more consistent.
* Usually a context is defined via a pair (`IOContext(io, :k=>v)`), but if another pair has to be pushed, one had to either add an IOContext call (`IOContext(IOContext(io, :k=>v), :x=>y)`) or to switch to kwargs syntax (`IOContext(io, k=v, x=y)`), none of which is satisfactory. * As a consequence of allowing passing multiple pairs, the method using keyword arguments is obsoleted (and hence deprecated). * Also, the method `IOContext(io, key, value)` is deprecated in favor of `IOContext(io, key=>value)`.
830b020 to
af2fd69
Compare
|
Wow, it became rare to see CI all green :) |
IOContext(io, :k=>v)),but if another pair has to be pushed, one had to either add an
IOContext call (
IOContext(IOContext(io, :k=>v), :x=>y)) or toswitch to kwargs syntax (
IOContext(io, k=v, x=y)), noneof which is satisfactory.
method using keyword arguments is obsoleted (and hence deleted).
IOContext(io, key, value)is deleted infavor of
IOContext(io, key=>value).I originally only wanted to allow
IOContext(io, pairs...), but in the process saw opportunities for simplification of the API, asIOContext(io, :key, value)andIOContext(io, key=value)were almost not used. I have no problem to restore them if they are deemed necessary.