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 (fix #29173) #29178

Merged
merged 3 commits into from
Oct 20, 2018
Merged

Conversation

rfourquet
Copy link
Member

@rfourquet rfourquet commented Sep 14, 2018

As was already discussed, this version is still a bit restrictive, in that we somehow require that the containers be equal after the call to copy!.
For example, should we call the following function copy! or copyto! ? copy_or_copyto!(s::Set, a::AbstractArray) = union!(empty!(s), a) ? This can also be discussed in the futureFuture.

@rfourquet rfourquet added the collections Data structures holding multiple items, e.g. sets label Sep 14, 2018
@fredrikekre
Copy link
Member

Include a deprecation for Future.copy!?

@rfourquet
Copy link
Member Author

Include a deprecation for Future.copy!?

Good point! Or maybe even keep it in there (without deprecation, as 1.x is meant to be stable) until 2.0 ?

@fredrikekre
Copy link
Member

Include a deprecation for Future.copy!?

Good point! Or maybe even keep it in there (without deprecation, as 1.x is meant to be stable) until 2.0 ?

Yea, I don't know what we have decided with here. There was talk about disabling deprecation warnings durin 1.X so that we can add deprecations but I don't know where we ended up. See also JuliaLang/Juleps#51

@StefanKarpinski
Copy link
Member

Eh, just leave it there with a comment for now. I for one don't have the energy for any more deprecations at this point.

@JeffBezanson
Copy link
Member

For example, should we call the following function copy! or copyto! ? copy_or_copyto!(s::Set, a::AbstractArray) = union!(empty!(s), a) ?

I don't think that fits either function. You might have to just write it out explicitly.

@@ -8,6 +8,9 @@ using Random

## copy!

# This has now been moved to Base (#29178), and should be deprecated in the
# next "deprecation phase".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cant these just call the Base methods now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a very good idea!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before pushing again and triggering CI, how about this new docstring:

"""
    Future.copy!(dst, src) -> dst

Copy `src` into `dst`.
This function has now been moved into `Base`, consider using `copy!(dst, src)` instead.
"""

@JeffBezanson JeffBezanson added this to the 1.1 milestone Sep 17, 2018
@JeffBezanson JeffBezanson merged commit c63ee65 into master Oct 20, 2018
@JeffBezanson JeffBezanson deleted the rf/copybang-to-base branch October 20, 2018 19:51
fredrikekre added a commit that referenced this pull request Dec 5, 2018
changes between Julia 1.0 and 1.1, including:

- Custom .css-style for compat admonitions.

- Information about compat annotations to CONTRIBUTING.md.

- NEWS.md entry for PRs #30090, #30035, #30022, #29978,
  #29969, #29858, #29845, #29754, #29638, #29636, #29615,
  #29600, #29506, #29469, #29316, #29259, #29178, #29153,
  #29033, #28902, #28761, #28745, #28708, #28696, #29997,
  #28790, #29092, #29108, #29782

- Compat annotation for PRs #30090, #30013, #29978,
  #29890, #29858, #29827, #29754, #29679, #29636, #29623,
  #29600, #29440, #29316, #29259, #29178, #29157, #29153,
  #29033, #28902, #28878, #28761, #28708, #28156, #29733,
  #29670, #29997, #28790, #29092, #29108, #29782, #25278

- Documentation for broadcasting CartesianIndices (#30230).
- Documentation for Base.julia_cmd().
- Documentation for colon constructor of CartesianIndices (#29440).
- Documentation for ^(::Matrix, ::Number) and ^(::Number, ::Matrix).

- Run NEWS-update.jl.

Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com>
Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>
fredrikekre added a commit that referenced this pull request Dec 5, 2018
changes between Julia 1.0 and 1.1, including:

- Custom .css-style for compat admonitions.

- Information about compat annotations to CONTRIBUTING.md.

- NEWS.md entry for PRs #30090, #30035, #30022, #29978,
  #29969, #29858, #29845, #29754, #29638, #29636, #29615,
  #29600, #29506, #29469, #29316, #29259, #29178, #29153,
  #29033, #28902, #28761, #28745, #28708, #28696, #29997,
  #28790, #29092, #29108, #29782

- Compat annotation for PRs #30090, #30013, #29978,
  #29890, #29858, #29827, #29754, #29679, #29636, #29623,
  #29600, #29440, #29316, #29259, #29178, #29157, #29153,
  #29033, #28902, #28878, #28761, #28708, #28156, #29733,
  #29670, #29997, #28790, #29092, #29108, #29782, #25278

- Documentation for broadcasting CartesianIndices (#30230).
- Documentation for Base.julia_cmd().
- Documentation for colon constructor of CartesianIndices (#29440).
- Documentation for ^(::Matrix, ::Number) and ^(::Number, ::Matrix).

- Run NEWS-update.jl.

Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com>
Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>
fredrikekre added a commit that referenced this pull request Dec 5, 2018
Addition of NEWS and compat admonitions for important changes between Julia 1.0 and 1.1, including:

- Custom .css-style for compat admonitions.

- Information about compat annotations to CONTRIBUTING.md.

- NEWS.md entry for PRs #30090, #30035, #30022, #29978,
  #29969, #29858, #29845, #29754, #29638, #29636, #29615,
  #29600, #29506, #29469, #29316, #29259, #29178, #29153,
  #29033, #28902, #28761, #28745, #28708, #28696, #29997,
  #28790, #29092, #29108, #29782

- Compat annotation for PRs #30090, #30013, #29978,
  #29890, #29858, #29827, #29754, #29679, #29636, #29623,
  #29600, #29440, #29316, #29259, #29178, #29157, #29153,
  #29033, #28902, #28878, #28761, #28708, #28156, #29733,
  #29670, #29997, #28790, #29092, #29108, #29782, #25278

- Documentation for broadcasting CartesianIndices (#30230).
- Documentation for Base.julia_cmd().
- Documentation for colon constructor of CartesianIndices (#29440).
- Documentation for ^(::Matrix, ::Number) and ^(::Number, ::Matrix).

- Run NEWS-update.jl.


Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com>
Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
collections Data structures holding multiple items, e.g. sets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants