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 nlohmann/json to 3.6.1 and improve JSON encoding of RuntimeInformation #213

Merged
merged 4 commits into from Apr 4, 2019
Merged

Conversation

niklas88
Copy link
Member

@niklas88 niklas88 commented Mar 20, 2019

This is the current stable version of nlohmann/json as of 19h ago.

As for for the JSON output for RuntimeInformation, this change preserves insertion order.

This ensures that converting from JSON directly to YAML results in a human readable format. Without it children are printed before their parents which makes the YAML very hard to read.

While the JSON, as included in QLever responses, is less readable, passing it through e.g. 'jq .' results in a readable performance digest now too.

nlohmann and others added 2 commits March 20, 2019 15:57
This ensures that converting from JSON directly to YAML results in
a human readable format. Without it children are printed before
their parents which makes the YAML very hard to read.

While the JSON, as included in QLever responses, is less readable,
passing it through e.g. 'jq .' results in a readable performance digest
now too.
@niklas88 niklas88 requested review from joka921 and removed request for joka921 March 20, 2019 17:30
@niklas88 niklas88 requested a review from joka921 March 20, 2019 17:35
@niklas88 niklas88 changed the title Update nlohmann/json to 3.6.0 and improve JSON encoding of RuntimeInformation Update nlohmann/json to 3.6.1 and improve JSON encoding of RuntimeInformation Mar 21, 2019
Copy link
Member

@joka921 joka921 left a comment

Choose a reason for hiding this comment

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

LGTM, I just have one question.

// https://github.com/nlohmann/json/issues/485#issuecomment-333652309
template <class K, class V, class dummy_compare, class A>
using fifo_map = nlohmann::fifo_map<K, V, nlohmann::fifo_map_compare<K>, A>;
using ordered_json = nlohmann::basic_json<fifo_map>;
Copy link
Member

Choose a reason for hiding this comment

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

It seems to work, but just out of curiosity: how is ordered_json a complete type here?
As I understand it fifo_map is still templated on <K, V, A> and I don't see an instantiation,
but later on we can write void foo(RuntimeInformation::ordered_json&j) without running into trouble.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think while fifo_map is indeed still templated and couldn't be used without template arguments, ordered_json is nlohmann::basic_json<fifo_map> which determines the K and V arguments.

@niklas88 niklas88 merged commit 24e7c22 into ad-freiburg:master Apr 4, 2019
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.

None yet

3 participants