Skip to content
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

Update MP provider #23

Closed
wants to merge 8 commits into from
Closed

Update MP provider #23

wants to merge 8 commits into from

Conversation

shyamd
Copy link
Contributor

@shyamd shyamd commented May 6, 2020

Updates the Materials Project provider

Copy link
Member

@CasperWA CasperWA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay, another! :)

A suggested change and a comment.

src/links/v1/providers.json Outdated Show resolved Hide resolved
src/links/v1/providers.json Outdated Show resolved Hide resolved
@CasperWA
Copy link
Member

@shyamd Could you update this either with the new "hosted" index meta-database in this repository or with a link to the un-versioned base URL of your own managed index meta-database? :)

@CasperWA
Copy link
Member

@shyamd The test failures are not necessarily due to your setup, but rather our new semver validation in the optimade-python-tools. It doesn't like the currently available index meta-databases for some reason (I believe it's the prefixed v).

Copy link
Member

@CasperWA CasperWA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes for name and description are simply suggestions. The latter since you essentially have the title/name repeated in the description.

The standard api_version value is wrong, I have put in a PR to update it, see #40.

Also, did you know your optimade server returns a 502?

"attributes" : {
"available_api_versions" : [
{
"version" : "1.0.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"version" : "1.0.0",
"version" : "1.0.0-rc.2",

"info",
"links"
],
"api_version" : "v1.0.0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"api_version" : "v1.0.0"
"api_version" : "1.0.0-rc.2"

{
"data" : [
{
"id" : "exmpl",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"id" : "exmpl",
"id" : "mp",

"type" : "links",
"attributes" : {
"name": "Materials Project",
"description": "The Materials Project: A database of computed materials properties for the discovery and design of novel materials.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"description": "The Materials Project: A database of computed materials properties for the discovery and design of novel materials.",
"description": "A database of computed materials properties for the discovery and design of novel materials.",

"id" : "exmpl",
"type" : "links",
"attributes" : {
"name": "Materials Project",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"name": "Materials Project",
"name": "The Materials Project",

"description": "The Materials Project: A database of computed materials properties for the discovery and design of novel materials.",
"base_url": "https://optimade.materialsproject.org",
"homepage": "https://materialsproject.org",
"link_type": "child"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"link_type": "child"
"link_type": "external"

"base_url": null,
"homepage": null,
"name": "Materials Project",
"description": "The Materials Project: A database of computed materials properties for the discovery and design of novel materials.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"description": "The Materials Project: A database of computed materials properties for the discovery and design of novel materials.",
"description": "A database of computed materials properties for the discovery and design of novel materials.",

"description": "",
"base_url": null,
"homepage": null,
"name": "Materials Project",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"name": "Materials Project",
"name": "The Materials Project",

@shyamd
Copy link
Contributor Author

shyamd commented Sep 24, 2020

I'm going to open a new PR since this is pretty stale.

@shyamd shyamd closed this Sep 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants