SimonEnsemble / PorousMaterials.jl

Julia package towards classical molecular modeling of nanoporous materials
GNU General Public License v3.0
50 stars 11 forks source link

Arni/doc brushup #18

Closed Surluson closed 6 years ago

Surluson commented 6 years ago

Added Arguments/Attributes and specifically Returns to a lot of docstrings.

GCMC.jl has yet to be updated, mostly because I wasn't sure how to describe some of the attributes in GCMCstats (like difference between n_samples and n). @ahyork or @SimonEnsemble, could you maybe update that when you get a chance.

I still haven't figured out how it would be best to write the docstrings for the multiple dispatch, so I'll be rolling out a doc_brushup2 soon, when I figure that out. All suggestions welcome!

Surluson commented 6 years ago

Oh, and there were some updates to functions in Misc.jl. All arguments and outputs involving atoms were working with AbstractString, but we changed the elements into Symbols a while ago so I fixed that

Surluson commented 6 years ago

The following case is why I decided to put AbstractString instead of String.

julia> function myfun1(mystring::String)
       println("This works")
       end
myfun1 (generic function with 1 method)

julia> function myfun2(mystring::AbstractString)
       println("This works")
       end
myfun2 (generic function with 1 method)

julia> filename = "ABUWOJ.cif"
"ABUWOJ.cif"

julia> filename_without_extension = split(filename, ".")[1]
"ABUWOJ"

julia> myfun1(filename_without_extension)
ERROR: MethodError: no method matching myfun1(::SubString{String})
Closest candidates are:
  myfun1(::String) at REPL[1]:2

julia> myfun2(filename_without_extension)
This works

The SubString{String} type is not a subtype of String,

julia> SubString{String} <: String
false

julia> SubString{String} <: AbstractString
true

julia> String <: AbstractString
true
SimonEnsemble commented 6 years ago

@Surluson thanks for educating me. I find the behavior of the AbstractString more intuitive. I guess they made the SubString so it could be a pointer to a position in an already existing String and save memory...

Surluson commented 6 years ago

Yeah I think that's correct.

(A relevant julia blogpost)

A quote from the blogpost:

All strings are subtypes of AbstractString, which provides most of the functionality.

SubString is the other string type included in Base. It is returned by the substring function, and by other functions such as split, and various regex matching. It holds a reference to the original string, as well as an offset and a length recording. This saves on allocating memory for the split-up string.