-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
define IOContext(io, pairs...) and remove IOContext(io; kw...) #23271
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. |
|
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.