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.

Sign up for GitHub

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

Jump to bottom

move Future.copy! to Base (fix #29173) #29178

Merged
merged 3 commits into from
Oct 20, 2018
Merged

move Future.copy! to Base (fix #29173) #29178

merged 3 commits into from
Oct 20, 2018

Conversation

rfourquet
Copy link
Member

@rfourquet rfourquet commented Sep 14, 2018 •
edited
Loading

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 domain: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 ?

@rfourquet rfourquet mentioned this pull request Sep 14, 2018
Closed
@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
Sponsor 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
Sponsor 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.

KristofferC
KristofferC reviewed Sep 14, 2018
View reviewed changes
stdlib/Future/src/Future.jl
@@ -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
Sponsor 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 Nov 30, 2018
@fredrikekre
News and compat annotation for #29178 (Future.copy! to Base).
1774e96
fredrikekre added a commit that referenced this pull request Dec 1, 2018
@fredrikekre
News and compat annotation for #29178 (Future.copy! to Base).
cd88535
fredrikekre added a commit that referenced this pull request Dec 1, 2018
@fredrikekre
News and compat annotation for #29178 (Future.copy! to Base).
cbb30ae
fredrikekre added a commit that referenced this pull request Dec 3, 2018
@fredrikekre
News and compat annotation for #29178 (Future.copy! to Base).
348a3e8
fredrikekre added a commit that referenced this pull request Dec 4, 2018
@fredrikekre
News and compat annotation for #29178 (Future.copy! to Base).
da8c7d7
fredrikekre added a commit that referenced this pull request Dec 4, 2018
@fredrikekre
News and compat annotation for #29178 (Future.copy! to Base).
3489e6e
fredrikekre added a commit that referenced this pull request Dec 5, 2018
@mortenpi @fredrikekre
Addition of NEWS and compat admonitions for important
cceb675 
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
@mortenpi @fredrikekre
Addition of NEWS and compat admonitions for important
ebbabf5 
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
@fredrikekre @mortenpi
Compat admonitions and NEWS for Julia 1.1 (#30230)
6175bd9 
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>
@vtjnash vtjnash mentioned this pull request Feb 12, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KristofferC KristofferC KristofferC left review comments

@JeffBezanson JeffBezanson JeffBezanson approved these changes

Assignees
No one assigned
Labels
domain:collections Data structures holding multiple items, e.g. sets
Projects
None yet
Milestone
1.1
Development

Successfully merging this pull request may close these issues.
5 participants
@rfourquet @fredrikekre @StefanKarpinski @JeffBezanson @KristofferC