-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add 2DMatpedia to the providers, with a static index meta-db #74
add 2DMatpedia to the providers, with a static index meta-db #74
Conversation
8feffcf
to
6889926
Compare
First, thank you for this PR to add in 2DMatpedia! :) I've checked out your implementation at http://optimade.2dmatpedia.org and there are some things that should probably be updated:
Concerning this specific PR I don't see any issues other than the incommensurate use of the provider prefix in your implementation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to clarify the "test" data, I wanted to at least at first try using the mongomock
backend using a static json file, and since I didn't see a way of providing a file, I merely replaced the optimade/server/data/test_structures.json
file. So yes, insert_test_data
is true, but that test data is an export from 2dmatpedia, adapted to the optimade specification
Right. If 2DMatpedia does not actually use MongoDB in the backend and the data is stored in a different kind of database you might need to setup your server differently, with a different If you want more assistance with this let either me, @ml-evs or @JPBergsma know and we will try to find some time to have a meeting to discuss how OPTIMADE Python tools can most effeciently be utilized for your implementation :) For now, you can surely do it the way you have - I would recommend to just avoid the server loading the |
it does (it was built with the materials project tools), but since I had to "massage" some data to make it conform to the optimade specification, it was simpler to do it on a file
I've just added the 2dmatpedia publication there
I am indeed using the implementation from optimade-python-tools, without any modifications, so that information is correct, no? |
Fair enough. This can be remedied by a special
Excellent.
Well, I'm guessing you're using a fork (or you should) or similar? Which you can then link to - maybe even the specific branch? As an example, it might be that you have calculated/converted a value of your structures wrongly, which is a specific 2DMatpedia issue, not an OPTIMADE Python tools issue :) |
Ok, I'll point that to the fork :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can also update the id
in the links.json
file if you wish, but it needs to be updated where I made the suggestion to become the official provider prefix.
src/links/v1/providers.json
Outdated
}, | ||
{ | ||
"type": "links", | ||
"id": "2dmatpedia", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"id": "2dmatpedia", | |
"id": "twodmatpedia", |
This represents the provider prefix as well :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about the folder name, does it also need to be twodmatpedia
then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @migueldiascosta, that might also make sense! Good to see that we have our first edge case in not allowing numbers in the prefix...
Also thanks for making this implementation with our tools! As @CasperWA suggested we are more than happy to meet to help streamline the process and make the package more ergonomic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is all good to go in my opinion - again, thank you for this contribution @migueldiascosta ! 👍
If at some point in the future you change from http
to https
remember to come back here and update the links.json
accordingly :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice to see a new provider! Welcome!
thanks (although I had opened this as a draft PR... guess we'll need to step up our efforts to improve what we are providing via optimade - maybe that was the point? :p ) btw, is that static index-metadb service down? all of those links in https://www.optimade.org/providers-dashboard/ return 404 (I did test running our own index with the optimade-python-tools and it seems to work, and we do have a few related databases, but at least for now we'd prefer using your static service, if possible) |
ah, ok, adding 'v1/links' to the url works (but then the link in the dashboard should point to something more informative? or just be the base_url string, not a clickable link...) |
Yeah I bumped it up (sorry). But several of the other providers here are already in a similar state. It's better to also register (and get it merged in) before having the fully fledged implementation on your side ready. In the end you can utilize the
That's what the service is here for :) You can always come back and update the |
Good point. I'll add an issue concerning this. Edit: Created issue here: Materials-Consortia/providers-dashboard#60. |
Also, as a note, I have updated the OPTIMADE Client hosted by Materials Cloud to support "minimized" structures, i.e., where not all structural information is valid, which was the case for you structures to begin with, I believe they have had more information added since then now - but you can peruse your OPTIMADE implementation through that client at https://materialscloud.org/optimadeclient :) |
yeah, I had started with the required ones, and then noticed that
Cool. Although I assumed this was the point, I wasn't aware that it already existed :) |
This catches me out all the time... I would be very happy to add redirects or a more informative 404 page, hopefully this is not too difficult with our current netlify setup. EDIT: although the problem is present when clicking on the links in the dashboard, I think the main problem is really that the static index meta-dbs hosted here do not have nice 404/landing pages, whereas most other implementations do (or at least they have a 404 that looks like an OPTIMADE implementation). |
No description provided.