Make robocrys and pmg addon packages optional

[munrojm]

• robocrys, pymatgen-analysis-diffusion, and pymagen-analysis-alloys are all now optional in emmet-core

[mkhorton]

How are you making packages optional when the document model depends on them, e.g. I believe pymatgen-analysis-alloys has AlloyPair in the document model definition?

[mkhorton]

robocrys I think is fine being optional because its document model does not depend upon any robocrys classes

[munrojm]

Yes, it does. I've just added a check to the beginning of the module import statements. That way if you go to import the model class it tells you to install the package if you don't have it. The modules involving addon packages are isolated in emmet-core so I figured this solution would be okay. I was under the assumption we absolutely don't want pymatgen to depend explicitly on pymatgen-analysis-diffusion or pymatgen-analysis-alloys.

[codecov-commenter]

Codecov Report

Merging #522 (48f7de7) into main (cd44a43) will decrease coverage by 0.01%.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##             main     #522      +/-   ##
==========================================
- Coverage   97.53%   97.52%   -0.02%     
==========================================
  Files         116      116              
  Lines       24135    24142       +7     
==========================================
+ Hits        23541    23544       +3     
- Misses        594      598       +4     

Impacted Files	Coverage Δ
emmet-core/emmet/core/robocrys.py	93.10% <77.77%> (-6.90%)	⬇️
emmet-core/emmet/core/mobility/migrationgraph.py	93.95% <80.76%> (-1.22%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[munrojm]

Happy to do something different if you feel there is a better solution.

[mkhorton]

I was under the assumption we absolutely don't want pymatgen to depend explicitly on pymatgen-analysis-diffusion or pymatgen-analysis-alloys.

That's fair; maybe. I think the main thing is to ensure that the API is still usable for people who don't have those installed, e.g. that we ensure that the error messages if you're trying to use the .alloys (or, similarly, any routes that depend on diffusion, e.g. wherever the migration hop API is going to live) get a nice error message which says:

To use the _____ API, please install the additional package ______ with a command like "pip install _______".

I think the error message on the import in emmet-core is ok, but wouldn't be sufficient for the client users, so perhaps we can just add another try/except ImportError in the client code.

[munrojm]

Yeah, my plan was to do just that and add another try-except block in the client.

[mkhorton]

I think this makes sense! I can vouch for pymatgen-analysis-alloys as being fairly light on the dependencies, but nevertheless it does add one (shapely) so I think this strategy is better.

Maybe we can slightly modify the API installation instructions to be pip install mp-api[all] or something for people who install the client directly.

[munrojm]

Maybe we can slightly modify the API installation instructions to be pip install mp-api[all] or something for people who install the client directly.

Sure, that sounds good to me!