Added support for queries against Materials Project#2097
Conversation
merkys
left a comment
There was a problem hiding this comment.
Please address the in-line comments on the code. The rest of the code looks OK. It would be great if you could supply a couple of tests for the parts of the code that do not need the network access.
|
Yes, in fact I also wanted some tests for the actual pull, but then we need the key. A public test key supplied by MP would be great (e.g. just pull one demo structure for instance) but I do not know of such functionality at the moment. Right now I think the only sensible test is to test that an invalid key returns the right error message or something similar as that is already in |
I guess we're OK with tests that do not interact with the API for now. We might test the generation of Results and Entry instances. |
|
One more note on plugin naming: I suggest using full names ( |
|
Yes, but I just followed what was there already. Totally agree on this topic. I also believe this should be done in the code, regardless of clutter and longer lines. Changed this now, but will leave the old stuff for now. |
|
The importer seems to have disappeared from this PR... Could be due to rename? |
|
Should be fine now. Also added the |
|
Great! It's very close to be finished. I have noted a few minor changes, and then we are ready to merge. |
Need to be able to pull data from the Materials Project. In this prototype, only the extraction of the structure is included. However, this can obviously be extended.