brainandforce / Electrum.jl

A Julian toolkit for solid-state chemical theory.
MIT License
31 stars 0 forks source link

Adding `InlineStrings` and conversion semantics for `NamedAtom` structs #146

Closed brainandforce closed 1 year ago

brainandforce commented 1 year ago

Previously, the NamedAtom struct was defined like such:

struct NamedAtom
    name::NTuple{8,Char}
    num::Int
    function NamedAtom(name::AbstractString, num::Integer)
        codepoints = ntuple(Val{8}()) do i
            i <= length(name) ? name[i] : '\0'
        end
        return new(codepoints, num)
    end
end

function Base.getproperty(atom::NamedAtom, p::Symbol)
    if p == :name
        l = findfirst(isequal('\0'), getfield(atom, p))
        return string(getfield(atom, p)[1:l-1]...)
    end
    return getfield(atom, p)
end

We used an NTuple{8,Char} to store a fixed-length C string so that NamedAtom can remain a bits type, and then overloaded Base.getproperty() and the constructors to deal with the fact that we want to treat the fixed string data as a Julia string. However, the InlineStrings.jl package defines fixed-length strings as their own data type, with all other Julia string operations supported, so this should significantly simplify the code. I've also allowed the name field to hold up to 15 characters, an improvement from the previous 8.

On top of that, I've included conversion semantics for NamedAtom structs, allowing them to be converted to strings or numbers easily.

brainandforce commented 1 year ago

It appears that this branch is failing checks because of method ambiguities that not only I introduced, but the InlineStrings.jl package introduced. I'll hold off on merging this for now.

brainandforce commented 1 year ago

Link to the issue I've opened on InlineStrings.jl regarding this problem: https://github.com/JuliaStrings/InlineStrings.jl/issues/64