adding ASE compatibility to Crystal class

[alex-rakowski]

Added two functions:

1. from_ASE creates Crystal from ase.Atom object
2. from_generic_file tries to create a Crystal using ase.io.read tested using a CIF file, but should add a lot of file types. This might need a better name.

[alex-rakowski]

After a little playing around I'm not sure how useful the generic file reader is, I forgot that standard xyz files don't contain cell params. I think the majority of the other supported file types are MD related. So I think I'll remove the generic function and add in from_prismatic instead.

[alex-rakowski]

Updated to have from_Ase and from_prismatic. I commented out the from_generic_file in case people wanted it back in, I can delete if preferred. Otherwise I think it should be good to go.

[alex-rakowski]

Updated to have from_Ase and from_prismatic. I commented out the from_generic_file in case people wanted it back in, I can delete if preferred. Otherwise I think it should be good to go.

[bsavitzky]

Thanks @alex-rakowski , this is a great idea! Two questions:

(1) dependency: right now you import ase in a try statement and have not modified the setup file. I definitely appreciate that this structure won't break the code for anyone. All that said -

• This might be worth just adding as a dependency. ase is a long-lived and well supported module.
• If we don't do this, we can add it as an optional dependency. I don't really love adding tons of optional dependency options, but I like it better than having some secret pseudo-dependencies where you can use certain methods iff you've installed the relevant packages yourself.
  What do you think?

(2) tests: our test suite is still far from robust...but its got better coverage in IO than anywhere else, and I'd love to ideally not reduce that coverage. The pipeline for testing file read stuff should be pretty easy to add (a) new file(s) + test(s) to. If you drop some new test files in our usual sample file drive and modify the file_ids and collection_ids dicts in io/google_drive_downloader.py to point to them you can make test files accessible to others (files should be SMALL, but shouldn't be an issue in this case). You could then add tests in test/test_nonnative_io using the patterns in the other two files in that directory.

[sezelt]

For an optional dependency that is not needed at the top level of the file, I think a better pattern is to move the import within the function that requires it. That way any import errors get raised at the appropriate moment.

[alex-rakowski]

I'm going to delete as its now a dependency

[sezelt]

Is all this extra logic necessary? If ase is missing it will raise a ModuleNotFoundError

[alex-rakowski]

I'm going to delete as its now a dependency

[alex-rakowski]

@sezelt Are you planning on creating a new PR for your CIF occupancy, adding into this one, or do you want to send me the code snippet and I can add it?

[alex-rakowski]

closing as this will be wrapped in to #538