Make compatible with AtomsBase 0.4.x

[cortner]

The interface in AB0.4 changed slightly. This PR is to update. I don't plan to support both AB 0.3 and 0.4, so this would become a breaking change to be registered as [email protected].

NB - tests still fail, I will ping once they pass.

[jameskermode]

Thanks, I'll take a look. Tests are currently failing so I can't merge as is.

[cortner]

Definitely not ready yet. I'll let you know.

[cortner]

tests pass now, but maybe this needs a brief discussion before merging. I cannot be sure that I've kept the spirit of some design choices. And there are also some arguments I think to simplify ExtXYZ a bit.

The main change in [email protected] that I struggled to incorporate here, is that atomic_number and atomic_symbol is now a property that is derives from species. Here, species(system, i) is the species of particle i. It could be anything, but if it is an atom then atomic_number(system, i) == atomic_number(species(system, i)) and so forth. The recommendation now is that a system stores ONLY the species. But none of the implementations so far actually follow this and that is causing a lot of hacking.

Either now or in the very near future I would prefer if ExtXYZ.Atoms stores the species either as a ChemicalSpecies (the AtomsBase recommendation) or as a Symbol, but not all three (Z, species and atomic_symbol)

Incidentally, this is the main reason I had to rewrite the test for equality. (which I'm afraid is an incomplete test now.)

[cortner]

If you would like to make changes to the PR, please just go ahead. I'm giving you push access to the fork.

[cortner]

I forgot to mention ... there are a few monkey-patches in the test files that I need to remove but I first have to make some bug fixes to AtomsBase.

[jameskermode]

Ok, will wait for those before merging.

I'll take a look at the species vs Z issue in the meantime. My original idea was to allow either or both to be present and to check for inconsistencies, raising an error if so. I'll look if this is still the case.

[cortner]

NOTE : The tests will (likely) fail now. To pass they require AtomsBase.jl#118 to be merged and registered. Once this is done then the current PR should be ready to merge.

[cortner]

There is also this discussion where it would be useful to get more input.

[jameskermode]

I'll be happy to merge this once the corresponding PR in AtomsBase has been merged.

[jameskermode]

Yes, this would be preferable

[cortner]

I filed an issue

[jameskermode]

Thanks. Shall I register as 0.2 or do we need to wait for AtomsBase?

[cortner]

I sent the registration request. Sorry I did this via an issue - forgot to add the registrator message to the commit.