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

move `Future.copy!` to Base #29173

Closed
chethega opened this issue Sep 13, 2018 · 5 comments

Comments

4 participants
@chethega
Copy link
Contributor

commented Sep 13, 2018

help?> copy!
search: copy! copyto! circcopy! unsafe_copyto! copy copysign deepcopy

  copy!(dst, src)

  In-place copy of src into dst. After the call to copy!, dst must be left
  equal to src, otherwise an error is thrown; this function appropriately
  resizes dst if necessary. See also copyto!.

Well, sounds convenient for a preallocated dst::Vector{T}, src::AbstractVector{T}. Alas,

julia> methods(copy!)
# 4 methods for generic function "copy!":
[1] copy!(dst::Random.MersenneTwister, src::Random.MersenneTwister) in Random at /build/julia/src/julia/usr/share/julia/stdlib/v1.0/Random/src/RNGs.jl:142
[2] copy!(dst::Random.DSFMT.DSFMT_state, src::Random.DSFMT.DSFMT_state) in Random.DSFMT at /build/julia/src/julia/usr/share/julia/stdlib/v1.0/Random/src/DSFMT.jl:37
[3] copy!(a::LibGit2.GitCredential, b::LibGit2.GitCredential) in LibGit2 at /build/julia/src/julia/usr/share/julia/stdlib/v1.0/LibGit2/src/gitcredential.jl:83
[4] copy!(dest::BitSet, src::BitSet) in Base at bitset.jl:58

I guess the docs went out of sync and copy! was deprecated for ( length(dst)==length(src) || resize!(dst, length(src)) ); copyto!(dst, src) at some point? I'm not sure whether the documented functionality really needs to exist, but either way, reality and doc should match.

For the next release, if copy! does not come back, can we rename the other copy! methods to something less prominent then? The BitSet! copy! probably needs to stay for now, since it is in base and would be breaking?

@fredrikekre

This comment has been minimized.

Copy link
Member

commented Sep 13, 2018

We just need to move Future.copy! to Base.

@chethega

This comment has been minimized.

Copy link
Contributor Author

commented Sep 13, 2018

Oops, sorry I missed that. So this is according to plan and the docs just updated faster than the actual stuff?

Close issue as PEBCAK, or move copy! now or change docs for the meantime?

@fredrikekre

This comment has been minimized.

Copy link
Member

commented Sep 13, 2018

I mean... the docs are not wrong, they don't say this works for vectors. Its just not yet implemented for vectors.

@JeffBezanson

This comment has been minimized.

Copy link
Member

commented Sep 13, 2018

I think we can move Future.copy! to Base right away. It could have been done for 1.0.

@JeffBezanson JeffBezanson changed the title copy! docs misleading move `Future.copy!` to Base Sep 13, 2018

@fredrikekre fredrikekre added this to the 1.1 milestone Sep 13, 2018

@rfourquet rfourquet self-assigned this Sep 14, 2018

@rfourquet

This comment has been minimized.

Copy link
Contributor

commented Sep 14, 2018

It could have been done for 1.0

I thought so, but 1.0 was out before I had time to ask whether it should go into 1.0 or 1.1 ;-)

rfourquet added a commit that referenced this issue Sep 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.