deprecate <<, >>, >>> on BitArray #30958
Labels
arrays
[a, r, r, a, y, s]
breaking
This change will break code
deprecation
This change introduces or involves a deprecation
Milestone
Comments
|
Would obviously be technically breaking. I guess the question is whether anyone is using this. If not, then this could potentially be considered a "minor change". |
|
I could have sworn that we did this, but looks like I was thinking of #23404. Bummer. I'm in full support of its deprecation — the only question is when we should do it. We should probably constrain ourselves to only remove the methods in 2.0. If that's the case, is it worth adding a deprecation to 1.2? Or should we wait until we're closer to 2.0? |
|
There's some discussion of the process here: JuliaLang/Juleps#51. These would become silent deprecations in 1.x, and either be removed or become noisy in 2.0. Unless, I guess, we can really be convinced nobody is using them. |
andrewjradcliffe
added a commit
to andrewjradcliffe/julia
that referenced
this issue
Jun 17, 2022
Preface: there are two distinct concepts here -- a constructor for `BitVector` from unsigned integers, and l/r-`shift[!]` methods which match the corresponding shifts on bit indices. When writing the code for `BitVector(x::Unsigned)`, I realized that the shift operators (`>>`, `>>>`, `<<`) have the opposite behavior w.r.t. the corresponding bitshifts. Odd, I thought, then found [this](JuliaLang#30958) -- sufficient motivation to write the l/r-`shift[!]` methods. Incidentally, these shifts happen to be quite useful when testing the behavior of the construction-from-unsigned, which is why these two concepts are initially being submitted as part of the same PR. Admittedly, I would normally have preferred to submit separate ideas as separate PRs so as to facilitate rejecting one or the other; my apologies for the inconvenience. Miscellaneous: the implementation of the l/r-`shift[!]` methods enables copy-free, in-place shifts on `BitVector`s, but it is not as efficient as it possibly could be -- there are some partial passes over the backing `Vector{UInt64}` which could probably be eliminated by properly re-writing `copy_chunks_rshift!`. I also notice the existence of a potentially dangerous but gainful opportunity for `@inbounds` in `_msk_rtol!`.
andrewjradcliffe
added a commit
to andrewjradcliffe/julia
that referenced
this issue
Jun 18, 2022
Preface: there are two distinct concepts here -- a constructor for `BitVector` from unsigned integers, and l/r-`shift[!]` methods which match the corresponding shifts on bit indices. When writing the code for `BitVector(x::Unsigned)`, I realized that the shift operators (`>>`, `>>>`, `<<`) have the opposite behavior w.r.t. the corresponding bitshifts. Odd, I thought, then found [this](JuliaLang#30958) -- sufficient motivation to write the l/r-`shift[!]` methods. Incidentally, these shifts happen to be quite useful when testing the behavior of the construction-from-unsigned, which is why these two concepts are initially being submitted as part of the same PR. Admittedly, I would normally have preferred to submit separate ideas as separate PRs so as to facilitate rejecting one or the other; my apologies for the inconvenience. Miscellaneous: the implementation of the l/r-`shift[!]` methods enables copy-free, in-place shifts on `BitVector`s, but it is not as efficient as it possibly could be -- there are some partial passes over the backing `Vector{UInt64}` which could probably be eliminated by properly re-writing `copy_chunks_rshift!`. I also notice the existence of a potentially dangerous but gainful opportunity for `@inbounds` in `_msk_rtol!`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I was surprised to find we still have these methods. Instead, we should have a
shift[!]function that works on any array. Using>>to move BitArray elements is arguably a pun, and I think the way we define it is arguably wrong:>>moves elements towards higher indices, while for integers>>moves bits towards lower bit indices.The text was updated successfully, but these errors were encountered: