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

xt::to_json and xt::from_json are specialized too much #1690

Closed
risa2000 opened this issue Jul 20, 2019 · 1 comment
Closed

xt::to_json and xt::from_json are specialized too much #1690

risa2000 opened this issue Jul 20, 2019 · 1 comment

Comments

@risa2000
Copy link

I am currently writing an app, which uses nlohmann::json and xtensor together. xtensor provides a serializer and deserializer functions xt::to_json and xt::from_json, with following prototypes:

template <class E>
enable_xexpression<E> to_json(nlohmann::json&, const E&);

template <class E>
enable_xcontainer_semantics<E> from_json(const nlohmann::json&, E&);

The nlohmann::json is basically synonym for nlohmann::basic_json<>, i.e. the basic type with all the specializations set to default. In my app however I need to use the non-default specialization with custom map, which preserves ordering (nlohmann::fifo_map), while the default uses std::map which does not preserve the insertion order.

So what I do in my code is something along this line:

using json = nlohmann::basic_json<nlohmann::fifo_map>

The code is more complex in the reality because of some bugs (nlohmann/json#485), but this is basically the idea. The problem is that this specialization does not compile with xt::to_json and xt::from_json default specialization.

It seems that an easy (and naive) fix could be rewriting the code in xjson.hpp to something along this line:

template<class J, class E>
enable_xexpression<E> to_json(J&, const E&);

template<class J, class E>
enable_xcontainer_semantics<E> from_json(const J&, E&);

I tested it on my code and this works fine. What I did not like, was that type J gives no information about its features - it is completely abstract.

So my second attempt was trying to define the type "un-specialized" enough, while still not losing everything, but the nlohmann::basic_json is quite a heavy weight:

template<template<typename U, typename V, typename... Args> class ObjectType =
         std::map,
         template<typename U, typename... Args> class ArrayType = std::vector,
         class StringType = std::string, class BooleanType = bool,
         class NumberIntegerType = std::int64_t,
         class NumberUnsignedType = std::uint64_t,
         class NumberFloatType = double,
         template<typename U> class AllocatorType = std::allocator,
         template<typename T, typename SFINAE = void> class JSONSerializer =
         adl_serializer>
class basic_json;

and the best thing I could come up with was this:

template<template<typename U, typename V, typename... Args> class M, class E>
enable_xexpression<E> to_json(nlohmann::basic_json<M>&, const E&);

template<template<typename U, typename V, typename... Args> class M, class E>
enable_xcontainer_semantics<E> from_json(const nlohmann::basic_json<M>&, E&);

which still loses all the other (default) template types in the specialization. It does however work with the custom map too.

Now I wonder if anyone knows a better way to tackle it or if some of the aforementioned changes will be acceptable into the library.

@JohanMabille
Copy link
Member

Fixed in #1691

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

No branches or pull requests

2 participants