AbstractSystem not exported any more in v0.4

[mfherbst]

I was a bit surprised to notice that AbstractSystem is no longer exported. I think this is a little unexpected (especially since FastSystem and FlexibleSystem are still exported), but I can't remember what we agreed on.

@cortner On purpose or a bug ?

[cortner]

On purpose. I probably forgot to flag this. Sorry about that. What would be the reason to export it?

[mfherbst]

For dispatches I use it in a couple of places.

[mfherbst]

e.g. in DFTK to distinguish when an AbstractSystem or some DFTK-internal thing is used.

[rkurchin]

I agree, I think it makes sense to export it for dispatch purposes, could imagine this being useful also for things like plotting recipes.

[cortner]

Why is exporting needed for dispatch? Simply using AtomsBase: AbstractSystem in any library that uses AbstractSystem.

I generally try to export only names that are "end-user" oriented. I do not equate exporting with public interface. I equate public interface with what is documented.

But as I'm thinking about it, then one use-case I see are update-constructors. Are there any left that are called via AbstractSystem? But tbh I would also prefer to rename those to something else. It doesn't feel natural.

[jgreener64]

I lean towards exporting it, as it is the central abstract type in the interface.

[mfherbst]

@cortner I see thanks for sharing your thoughts. I agree with your philosophy.

But I would argue that AbstractSystem is user-oriented. Update constructor is a potential case, but I also think about another case of a user who is faced with a non AB datastructure, that they want to use in the AB ecosystem. For that standardising on a convert(AbstractSystem, datastructure) pattern is quite useful as the implementor of this method can still choose which AB backend to convert to ... and its not sth the user needs to know.

I have used this pattern in ASEconvert and in the Chemfiles so far, where the way to use python datastructures in julia (pyconvert(AbstractSystem, ase_atoms) also would benefit from this exported for convenience.

[cortner]

I can see some arguments for update constructors but don't like using AbstractSystem as the symbol. I do not like the arbitrary selection of a default implementation.

We can export it for now and discuss this again at the molssi workshop. @mfherbst if you make a quick PR that just adds an export line then just go ahead and merge and register without review.

[cortner]

fixed by #114