Skip to content

OPTIMADE JSON Lines specification appendix#531

Merged
ml-evs merged 20 commits intodevelopfrom
ml-evs/jsonlines-draft
Mar 7, 2025
Merged

OPTIMADE JSON Lines specification appendix#531
ml-evs merged 20 commits intodevelopfrom
ml-evs/jsonlines-draft

Conversation

@ml-evs
Copy link
Member

@ml-evs ml-evs commented Jul 19, 2024

This PR begins the work on #471, by defining the conventions on top of JSON Lines for deserializing an entire OPTIMADE API into a file.

@ml-evs ml-evs changed the title [WIP] OPTIMADE JSON Lines specification appendix OPTIMADE JSON Lines specification appendix Jul 29, 2024
Fix code block formatting

Update optimade.rst

Apply suggestions from code review

Apply suggestions from code review

Try to fix formatting again

Fix formatting and add note about provider fields
@ml-evs ml-evs force-pushed the ml-evs/jsonlines-draft branch from 6929f10 to c5f1f53 Compare July 29, 2024 20:51
@ml-evs ml-evs force-pushed the ml-evs/jsonlines-draft branch from 96b0231 to 78e158c Compare July 29, 2024 21:09
@ml-evs ml-evs marked this pull request as ready for review July 30, 2024 16:55
@ml-evs ml-evs requested a review from eimrek July 30, 2024 16:55
Copy link
Member

@eimrek eimrek left a comment

Choose a reason for hiding this comment

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

Thanks @ml-evs, and sorry for the delays. Now had some time to take a look. I like the proposal and it can be used for optimade-maker as well. But I wrote some comments for consideration.

Co-authored-by: Kristjan Eimre <eimrek@users.noreply.github.com>
ml-evs and others added 2 commits January 23, 2025 17:34
@ml-evs
Copy link
Member Author

ml-evs commented Jan 23, 2025

CI is blocked by #536, but perhaps you could take another look when you get a chance @eimrek? I'll invite some other reviewers too.

@ml-evs ml-evs requested review from eimrek, merkys and rartino January 23, 2025 18:04
Copy link
Contributor

@rartino rartino left a comment

Choose a reason for hiding this comment

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

Looks good; and as we said during the web meet, very important extension.

However, how are relationships encoded? Maybe it would be nice if the example included a structure that has a relationship to a reference so one can see explicitly how that is encoded?

@ml-evs ml-evs requested a review from rartino February 3, 2025 13:59
@ml-evs
Copy link
Member Author

ml-evs commented Feb 17, 2025

An additional question about custom prefixes, that is (still) not clear to me, is what should happen when provider1 creates a JSONL file, and provider2 subsequently serves this.

Should all _provider1 prefixes be replaced by _provider2?

or would _provider1 prefixes be retained and this would fall under 3.5.2?

I think "serving" is an entirely different thing; provider2 should decide whether to re-host the properties with their own prefix, or point to the provider1 definition if it is available. It's a bit tricky to write this out (also somewhat addressed by d2c095a) -- somehow there are properties that are going to be namespace scoped to just the file in question as the file is the "provider". They can then be served with any prefix as long as they are consistent.

@ml-evs ml-evs force-pushed the ml-evs/jsonlines-draft branch from e9ff554 to 2f9e2d7 Compare February 17, 2025 18:52
ml-evs and others added 3 commits February 25, 2025 14:12
Co-authored-by: Kristjan Eimre <eimrek@users.noreply.github.com>
Co-authored-by: Kristjan Eimre <eimrek@users.noreply.github.com>
@ml-evs ml-evs requested review from eimrek, merkys and rartino March 3, 2025 22:26
@ml-evs
Copy link
Member Author

ml-evs commented Mar 3, 2025

Thanks for the comments @eimrek, @merkys and @rartino -- I think I've clarified them now so this should be ready to go again.

Co-authored-by: Kristjan Eimre <eimrek@users.noreply.github.com>
@ml-evs ml-evs requested a review from eimrek March 4, 2025 14:29
vaitkus
vaitkus previously approved these changes Mar 7, 2025
merkys
merkys previously approved these changes Mar 7, 2025
Copy link
Member

@merkys merkys left a comment

Choose a reason for hiding this comment

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

I have no objections for this to be merged. I have myself encountered a need to store database subsets in OPTIMADE format, and this format seems to be just what I need.

Co-authored-by: Antanas Vaitkus <antanas.vaitkus90@gmail.com>
Co-authored-by: Andrius Merkys <andrius.merkys@gmail.com>
@ml-evs ml-evs dismissed stale reviews from merkys and vaitkus via 9abb083 March 7, 2025 16:05
@ml-evs ml-evs requested review from eimrek, merkys and vaitkus March 7, 2025 16:06
@ml-evs ml-evs merged commit 331f37e into develop Mar 7, 2025
4 checks passed
@ml-evs ml-evs deleted the ml-evs/jsonlines-draft branch March 7, 2025 16:51
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

Comments