Add `See also:` , remove description of other functions in base.array.jl #23789

ChristianKurz · 2017-09-20T06:43:37Z

KristofferC · 2017-09-20T06:46:36Z

base/array.jl

@@ -1080,6 +1080,8 @@ end
 Remove an item in `collection` and return it. If `collection` is an
 ordered container, the last item is returned.

+See also: shift!, splice!


[`shift!`](@ref)

etc. to add links in the HTML documentation and code quote the identifier.

fredrikekre · 2017-09-20T07:24:20Z

base/array.jl

@@ -1080,6 +1080,8 @@ end
 Remove an item in `collection` and return it. If `collection` is an
 ordered container, the last item is returned.

+See also: [`shift!`](@ref), [`splice!`](@ref)


What about push!

Push! adds elements, whereas the other functions remove elements. I think, See also:-lists should be rather minimal: They reduce the need to search the documentation but it's no replacement for a proper search.

A thesaurus lists both similar words and antonyms for a reason – knowing about push! is useful if you're looking at pop!. From push! you could potentially find all the other element addition functions.

garrison · 2017-09-21T01:24:52Z

base/array.jl

@@ -1147,6 +1149,8 @@ end

 Remove the first `item` from `collection`.

+See also: [`pop!`](@ref), [`splice!`](@ref)


What about unshift!?

`See also`-Lists now contain up to two related functions and antonym at the end Docs of `append!` were not in line with `prepend!`.

garrison · 2017-09-21T15:20:52Z

base/array.jl

@@ -982,6 +991,10 @@ julia> prepend!([3],[1,2])
 2
 3
 ```
+
+Use [`unshift!`](@ref) to add individual items to `collection` which are not already
+themselves in another collection. The result is of the preceding example is equivalent to


I find the phrase "which are not already themselves in another collection" confusing. I think I know what you are getting at, but I would remove it.

garrison · 2017-09-21T15:22:03Z

base/array.jl

+
+Use [`unshift!`](@ref) to add individual items to `collection` which are not already
+themselves in another collection. The result is of the preceding example is equivalent to
+`unshift!([1, 2, 3], 4, 5, 6)`.


The previous example (prepend!([3],[1,2]) I believe) actually has a different result. And there is a duplicate "is" in the above sentence.

I think the sentence is more clear, with out the "which are not already..." part.

nalimilan · 2017-09-22T11:27:59Z

base/array.jl

@@ -895,9 +898,8 @@ julia> push!([1, 2, 3], 4, 5, 6)
 6
 ```

-Use [`append!`](@ref) to add all the elements of another collection to


Please don't change unrelated lines. This applies to other places below.

nalimilan · 2017-09-22T11:33:08Z

base/array.jl

 ```
+
+Use [`unshift!`](@ref) to add individual items to `collection`. The result of


I'd rather put this in the "See also" paragraph, rather than after the examples, where people might not find it (examples can be long). No need to refer to a particular example, just say "prepend!(x, y) is equivalent to...".

I removed these sentences in favor of a "See also:" entry. We opted against further descriptions in "See also" earlier.

I removed all descriptions of other functions and inserted an entry in `See also:` instead. This is related to my first commits' proposal of using a link-description list as `See also:`. I also removed the redundant description in `splice!` for removing items. I opted to remove it from the single-item method, as a (negative) range is used to remove items.

fredrikekre · 2017-09-25T09:09:47Z

base/array.jl

@@ -916,9 +916,11 @@ function push!(a::Array{Any,1}, @nospecialize item)
 end

 """
-    append!(collection, collection2) -> collection.
+    append!(a::Vector, items) -> collection.


Why this change?

a has to be a vector for append! to work. Of course, items could stay collection, but this wording is consistent with the other docstrings. Perhaps collection would be clearer, though, as items may be interpreted as several seperate items and append! expects a collection of items...

julia> append!([1 2 3], 1) ERROR: MethodError: no method matching append!(::Array{Int64,2}, ::Int64) You might have used a 2d row vector where a 1d column vector was required. Note the difference between 1d column vector [1,2,3] and 2d row vector [1 2 3]. You can convert to a column vector with the vec() function. Closest candidates are: append!(::Array{T,1} where T, ::Any) at array.jl:637 julia> append!([1,2,3], 1) 4-element Array{Int64,1}: 1 2 3 1 julia> append!([1,2,3], 1, 2) ERROR: MethodError: no method matching append!(::Array{Int64,1}, ::Int64, ::Int64) Closest candidates are: append!(::Array{T,1} where T, ::Any) at array.jl:637 append!(::Array{#s45,1} where #s45, ::AbstractArray{T,1} where T) at array.jl:630 julia> append!([1,2,3], [1 2]) 5-element Array{Int64,1}: 1 2 3 1 2

This change should go to a separate commit or PR, and it should use AbstractVector. I also wonder whether this isn't on purpose, since non-AbstractVector collections defined in packages could define methods and it would be silly to repeat the information in another docstring.

fredrikekre · 2017-09-25T09:09:50Z

base/array.jl

 # Examples
 ```jldoctest
 julia> prepend!([3],[1,2])
 3-element Array{Int64,1}:
 1
 2
 3
+
+julia> prepend!([1, 2, 3], [4, 5, 6])


What does this extra example show that the one just above doesn't?

this is a remnant of earlier edits, will remove.

Edit: Done, also removed the duplicate example for append!

nalimilan · 2017-09-25T09:32:57Z

base/array.jl

@@ -1212,6 +1229,8 @@ end
 Remove the item at the given `i` and return the modified `a`. Subsequent items
 are shifted to fill the resulting gap.

+See also: [`splice!`](@ref)


Maybe also pop! and unshift!?

nalimilan · 2017-09-25T09:34:04Z

base/array.jl

@@ -1316,6 +1335,8 @@ Subsequent items are shifted left to fill the resulting gap.
 If specified, replacement values from an ordered
 collection will be spliced in place of the removed item.

+See also: [`pop!`](@ref), [`shift!`](@ref), [`insert!`](@ref)


Also deleteat! (i.e. links should generally go both directions, right?).

Yeah, right. I think this would be easier if See also: lists would be generated automatically from some kind of relation-table.

I left these out, as deleteat! does not return the removed element, but sure - these are related and should be in See also:.

Add See also: to pop!, shift! and splice!

c85de3b

JuliaLang#23788

KristofferC reviewed Sep 20, 2017

View reviewed changes

added @ref syntax

9d8a240

fredrikekre reviewed Sep 20, 2017

View reviewed changes

garrison reviewed Sep 21, 2017

View reviewed changes

Added antonyms, updated docs of append!

590e74a

`See also`-Lists now contain up to two related functions and antonym at the end Docs of `append!` were not in line with `prepend!`.

garrison reviewed Sep 21, 2017

View reviewed changes

Added matching example, corrected sentence

14a3f74

I think the sentence is more clear, with out the "which are not already..." part.

nalimilan reviewed Sep 22, 2017

View reviewed changes

ChristianKurz changed the title ~~Add See also: to pop!, shift! and splice!~~ Add See also: , remove description of other functions in base.array.jl Sep 25, 2017

fredrikekre reviewed Sep 25, 2017

View reviewed changes

nalimilan reviewed Sep 25, 2017

View reviewed changes

removed duplicate examples, added see also: items for deleteat!

751e348

garrison mentioned this pull request Sep 30, 2017

Rename shift! and unshift! ? #23902

Closed

ChristianKurz closed this Aug 17, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `See also:` , remove description of other functions in base.array.jl #23789

Add `See also:` , remove description of other functions in base.array.jl #23789

ChristianKurz commented Sep 20, 2017

KristofferC Sep 20, 2017 •

edited

Loading

fredrikekre Sep 20, 2017

ChristianKurz Sep 20, 2017

StefanKarpinski Sep 20, 2017

garrison Sep 21, 2017

garrison Sep 21, 2017

garrison Sep 21, 2017

nalimilan Sep 22, 2017

nalimilan Sep 22, 2017

ChristianKurz Sep 25, 2017

fredrikekre Sep 25, 2017

ChristianKurz Sep 25, 2017 •

edited

Loading

nalimilan Sep 25, 2017

fredrikekre Sep 25, 2017

ChristianKurz Sep 25, 2017 •

edited

Loading

nalimilan Sep 25, 2017

nalimilan Sep 25, 2017

ChristianKurz Sep 25, 2017 •

edited

Loading

		@@ -1147,6 +1149,8 @@ end

		Remove the first `item` from `collection`.

		See also: [`pop!`](@ref), [`splice!`](@ref)

		```

		Use [`unshift!`](@ref) to add individual items to `collection`. The result of

Add See also: , remove description of other functions in base.array.jl #23789

Add See also: , remove description of other functions in base.array.jl #23789

Conversation

ChristianKurz commented Sep 20, 2017

KristofferC Sep 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChristianKurz Sep 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChristianKurz Sep 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChristianKurz Sep 25, 2017 • edited Loading

Choose a reason for hiding this comment

Add `See also:` , remove description of other functions in base.array.jl #23789

Add `See also:` , remove description of other functions in base.array.jl #23789

KristofferC Sep 20, 2017 •

edited

Loading

ChristianKurz Sep 25, 2017 •

edited

Loading

ChristianKurz Sep 25, 2017 •

edited

Loading

ChristianKurz Sep 25, 2017 •

edited

Loading