docs: revise and expand ImmutableDict docstring. #58445
docs: revise and expand ImmutableDict docstring. #58445rfritz wants to merge 7 commits intoJuliaLang:masterfrom
ImmutableDict docstring. #58445Conversation
|
Thanks for the contribution, but can we please have a more descriptive title? The current one is rather cryptic. |
|
Ooops! That was pretty obscure. I've updated the commit message on dict.jl following the procedure (and wincing at the warnings) given in "Changing a commit message." Will that serve? |
giordano
changed the title
giordano
changed the title
ImmutableDict to list of dictionary types
|
Ok, I changed the title of the PR based on commit message. As reference for next time, you can do it by clicking on the "Edit" button in the top right corner of the page. |
rfritz
changed the title
ImmutableDict to list of dictionary typesImmutableDict docstring.
|
oh, duh. thanks for the correction and information. |
MasonProtter
left a comment
MasonProtter
left a comment
There was a problem hiding this comment.
Just some minor comments / tweaks
Co-authored-by: Mason Protter <mason.protter@icloud.com>
|
Thanks for continuing to work on this, @rfritz! FYI, in the future you can continue using the previous pr and don't have to open a new one, that way all the conversation stays in the same place. We can keep working here now, though :) |
Co-authored-by: Mason Protter <mason.protter@icloud.com>
|
Is there anything else I need to do on this? Or is it just a matter of waiting until it's picked up in a release? |
Co-authored-by: Mosè Giordano <765740+giordano@users.noreply.github.com>
fingolfin
left a comment
fingolfin
left a comment
There was a problem hiding this comment.
Just discovered this PR. Thanks for working on this, but I do think there is some more work needed. Sorry for taking so long to get you feedback :-(
base/dict.jl
Outdated
| which is optimal for small dictionaries that are constructed over many individual insertions. | ||
| Note that it is not possible to remove a value, although it can be partially overridden and hidden | ||
| by inserting a new value with the same key. | ||
| `ImmutableDict{K,V}()` |
There was a problem hiding this comment.
This ImmutableDict{K,V}() appearing is a bit surprising, is it intentional?
There was a problem hiding this comment.
Thanks. I've reorganized the text so that the constructor signatures appear immediately above the description of what they do; I hope that's appropriate style.
The jldoctest will take a while; I've never done that before
|
|
||
| julia> ctypes = Base.ImmutableDict("char"=>:char, "int"=>:int) | ||
| Base.ImmutableDict{String, Symbol} with 2 entries: | ||
| "int" => :int | ||
| "char" => :char | ||
|
|
||
| julia> ctypes = Base.ImmutableDict(ctypes, "float"=>:float, "double"=>:double) | ||
| Base.ImmutableDict{String, Symbol} with 4 entries: | ||
| "double" => :double | ||
| "float" => :float | ||
| "int" => :int | ||
| "char" => :char | ||
|
|
||
| julia> viewparameters = Base.ImmutableDict{String,Any}() | ||
| Base.ImmutableDict{String, Any}() | ||
|
|
||
| julia> viewparameters = Base.ImmutableDict( | ||
| viewparameters, "type"=>"perspective", "direction"=>(0, 0, 1)) | ||
| Base.ImmutableDict{String, Any} with 2 entries: | ||
| "direction" => (0, 0, 1) | ||
| "type" => "perspective" |
There was a problem hiding this comment.
This really should be in a jldoctest block so that the examples are tested as part of our automated testing, and deviations in the outputs are noticed and fixed.
Move the constructor signature descriptions to be just above the documentation for the constructor.
6 participants
I had difficulty using the ImmutableDict type based on the documentation. After studying the code and reviewing advice on the Discourse site, I wrote this. This is the second version; I have made it match the style of the existing dictionary documentation and removed the tutorial content.