Element can't be found when using String for AtomID

[louisponet]

AtomsBase.jl/src/interface.jl

Line 91 in 652f916

element(id::Union{Symbol,AbstractString,Integer}) = PeriodicTable.elements[id]

MWE:

Atom("Si", zeros(3)u"\AA")

[mfherbst]

Yes, because the convention is in PeriodicTable that strings represent names of elements and Symbols represent the element symbol, e.g. this works:

Atom("silicon", zeros(3)u"Å")

or this

Atom(Symbol("Si"), zeros(3)u"Å")

But clearly the current behaviour is confusing. At least it should display a better error. I see these options for improving the situation:

1. Catch the KeyError, check it's a string and display a better error message (e.g. "maybe you wanted to pass this as a symbol"). The very least we should do, makes it clear what PeriodicTables expects.
2. Catch the KeyError, check it's a string and automatically convert to a Symbol to retry. This is convenient, but it feels like hiding some details from the user and could become potentially confusing (as our behaviour then very much deviates from PeriodicTables)
3. Generally people don't denote elements by their name. So maybe we should ditch support for Atom("silicon", zeros(3)u"\AA") completely and either drop supporting element(id::AbstractString) or just always convert strings to symbols in the element function (the latter would also be a breaking deviation from PeriodicTables)

I lean towards 1 or 3. I think 2 might lead to confusion. With respect to 3 I think keeping compatibility with PeriodicTables is probably a good idea (so I would completely remove the support for the AbstractString version).

What are other's thoughts?

[jgreener64]

Slight preference for 1, then 3.

[rkurchin]

I agree with Joe – I've also come across this point of confusion a few times now, so I would view 1 as a bare minimum here to smooth out the user experience.

[mfherbst]

I made a proposal in #72. Let me know what you think.