Add Pourbaix client

[rkingsbury]

• Add get_pourbaix_entries to MPRester with an equivalent call signature to the legacy pymatgen method
• Add support for calculating solid free energies via GibbsComputedStructureEntry
• Update the source of ion data to point to the MPContribs project
• Add get_ion_reference_data and get_ion_entries methods to make it a little easier and more transparent for users to customize Pourbaix diagrams
• Port legacy tests from pymatgen
• Add more tests

Contributor Checklist

• I have broken down my PR scope into the following TODO tasks
  • see above
• I have run the tests locally and they passed.
• I have added tests, or extended existing tests, to cover any new features or bugs fixed in this PR

TODO

• Various technical cleanups. See TODO comments throughout the code
• Make tests of get_pourbaix_entries more thorough

[mkhorton]

I'd be down with merging this as-is and improving with a future PR, concur? This is great otherwise, thank you(!)

[mkhorton]

You could use Union[str, Tuple] instead, which is hashable.

[rkingsbury]

I'd prefer to keep the call signature consistent with get_pourbaix_entries and other methods.

[mkhorton]

To keep it consistent with the legacy method? It has to be tuple/hashable if you want to use @lru_cache, it cannot be list. Change is worth it imo to make the function cachable.

[rkingsbury]

to keep consistent with get_entries or get_entries_in_chemsys. Users expect to be able to specify a chemsys as either a string Li-Fe-O or a list ['Li','Fe','O'], right? I don't recall seeing anywhere where we specify the chemsys as a tuple

[mkhorton]

Right, but we can change that. Or you can make this a private method that accepts a tuple and the public method accepts a list and converts it. Either way. But I think it's an important method to cache.

[rkingsbury]

Sure, no argument at all with that approach; I just want to keep the user-facing arguments consistent

[mkhorton]

Yes. You can query for a formula containing a substring with formula__contains or any of the data fields, see https://contribs-api.materialsproject.org/#/contributions/get_entries for syntax.

[rkingsbury]

OK. What we would really need here is the equivalent of mongo $in, i.e. {'MajElements`: {"$in": chemsys}}. As best I can tell that isn't possible.

[mkhorton]

I refer that to @tschaume :)

[tschaume]

You should be able to use data__MajElements__in, e.g. https://contribs-api.materialsproject.org/contributions/?project=ion_ref_data&data__MajElements__in=Li,B&_fields=identifier,data.MajElements

[rkingsbury]

I'd be down with merging this as-is and improving with a future PR, concur? This is great otherwise, thank you(!)

Thanks @mkhorton! I'm good with this as is now. Please check the test failures though; there seems to be something wrong with the api key setting in the test environment but I don't think it's related to anything in this PR. Not 100% sure though. There are a ton of these errors:

 E               mp_api.core.client.MPRestError: REST query returned with error status code 401 on URL https://api.materialsproject.org/xas/?edge=K&limit=1&fields=edge with message:
E               Response {
E                 "message":"No API key found in request"
E               }

[munrojm]

The tests should pass once merged. Can you confirm they are all passing locally @rkingsbury ?

[rkingsbury]

The tests should pass once merged. Can you confirm they are all passing locally @rkingsbury ?

Locally, I have one failure in tests/charge_density/test_client.py and one in tests/test_mprester.py which both appear related to the charge density endpoint. I can't imagine is related to this

E               mp_api.core.client.MPRestError: REST query returned with error status code 403 on URL https://api.materialsproject.org/charge_density/?task_ids=mp-655585&task_ids=mp-1057373&task_ids=mp-1059589&task_ids=mp-1440634&task_ids=mp-1791788&limit=10&fields=last_updated%2Ctask_id%2Cfs_id with message:
E               Response {
E                 "message":"You cannot consume this service"
E               }

[mkhorton]

Ok, I'll go ahead and merge and we can fix up in an additional PR. Thanks again!

[tschaume]

@rkingsbury The You cannot consume this service message is triggered by the charge_density endpoint being open only to staff. I can add you if needed.

[rkingsbury]

@rkingsbury The You cannot consume this service message is triggered by the charge_density endpoint being open only to staff. I can add you if needed.

that would be great @tschaume , thanks. I might actually make use of that soon as part of the SCAN work.