RFC: use keyword args for setting properties #632
Conversation
|
This is definitely the correct way to go. I wonder about switching from strings to symbols. Any particular reason to change that? I'm not sure on how best to handle the breaking change, but I definitely want to see this in. |
|
The key type of the slurped kwargs is a symbol so I just wanted to avoid an unnecessary conversion to a string. This changes the case where you set a property explicitly e.g. |
kleinhenz
force-pushed
the
kwargs_props
branch
from
9146381
to
886c21b
Compare
|
Makes sense. Would it be ok to merge this and make a breaking release? I don't see an obvious way of making this non-breaking or adding a deprecation message given how the previous structure was coded. |
|
Perhaps we should add a major note in the README |
kleinhenz
force-pushed
the
kwargs_props
branch
2 times, most recently
from
23822e2
to
570d632
Compare
|
Yeah a breaking release is probably the way to go. I can't think of a great way to deprecate this either. I added a |
|
We'll also need to update the syntax in the |
kleinhenz
force-pushed
the
kwargs_props
branch
from
570d632
to
ceb1d87
Compare
|
Updated the documentation for this. The new docs look great BTW! |
|
I think I will first merge 978050c after reviewing it. Make a minor release. Then merge this and make a breaking release. Any objections? |
|
Also just want to say this is a massive improvement over the very old style of 'keyword' handling that HDF5 is using (very legacy) |
|
Makes sense to me. It also probably makes sense to try merge #643 before the minor release to get the ci passing again. |
|
On second thought I think we should just merge this first and the following PRs and include them in the major release. That way if code magically breaks due to someone relying on internals it won't be an issue. |
The defaults were removed in JuliaIO#632.
The defaults were removed in JuliaIO#632.
* Add deprecated bindings for backwards compatibility with ecosystem packages Namely, get the tests of JLD.jl and MAT.jl to pass again. * Deprecations for #632 - keywords instead of property lists * Deprecations for #652 - generic read * Deprecate {d,a}_{create,write} methods with property lists The calls should use keyword properties instead.
JuliaIO/HDF5.jl#632 switched properties to keyword arguments.
I decided to try this while looking at #631. This replaces the current method of setting properties using varargs with keyword arguments. This is breaking so we should discuss if it is worth it. In #462 where the current behavior was introduced it looks like this wasn't done because the behavior of keyword arguments was still stabilizing in julia. Now that julia has stabilized I think this is a better interface.