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

Deserialize std::map with std::nan #1154

Closed
giumas opened this issue Jul 4, 2018 · 10 comments
Closed

Deserialize std::map with std::nan #1154

giumas opened this issue Jul 4, 2018 · 10 comments
Labels
solution: proposed fix a fix for the issue has been proposed and waits for confirmation

Comments

@giumas
Copy link

giumas commented Jul 4, 2018

  • What is the issue you have?
    Using the 'develop' branch, I get: [json.exception.type_error.302] type must be number, but is null when I deserialize an std::map with std::nan as pair value.

  • Please describe the steps to reproduce the issue. Can you provide a small but working code example?

#include <iostream>
#include <iomanip>
#include <map>

#include "rawdata/ext/json/json.hpp"
#include <fstream>

using namespace std;
using json = nlohmann::json;

int main()
{
	// a map with a nan-value pair
	map<int64_t, double> test_map;
	//test_map[123] = 1.2342;
	test_map[434] = std::nan("");
	test_map[999] = 243.12;

	// storage file
	string file_path = "test.json";

	// serialization
	json j;
	j["data"] = test_map;
	string json_str = j.dump(3);
	cout << "json str: " << json_str << endl;
	// write to disk
	std::ofstream ofs( file_path );
	if(!ofs)
	{
		cerr << "issue in opening " << file_path << endl;
		exit( -1 );
	}
	ofs << json_str;
	ofs.close();

	// de-serialization
	std::ifstream ifs( file_path );
	if(!ifs)
	{
		cerr << "issue in opening " << file_path << endl;
		exit( -1 );
	}
	json j2;
	ifs >> j2;
	json_str = j2.dump();
	ifs.close();
	try
	{
		map<int64_t, double> reloaded_map = j2["data"].get<std::map<int64_t, double>>();
		for(const auto& pair: reloaded_map)
		{
			cout << "- " << pair.first << ": " << pair.second << endl;
		}
	}
	catch(const std::exception& e)
	{
		cerr << "issue in parsing json: " << e.what() << endl;
	}
}
  • What is the expected behavior?
    No throwing an exception.

  • Which compiler and operating system are you using? Is it a supported compiler?
    Visual Studio 2015

  • Did you use a released version of the library or the version from the develop branch?
    develop branch.

@nlohmann
Copy link
Owner

nlohmann commented Jul 4, 2018

The reason for this is that the double value NaN is serialized to null as JSON does not have a NaN value. This behavior was motivated by JavaScript, e.g.

> a = [0/0, 1/0, -1/0]
[ NaN, Infinity, -Infinity ]
> JSON.stringify(a)
'[null,null,null]'

Parsing null then succeeds, but cannot be assigned to double. I am not sure what the expected behavior would be.

  • Throwing when trying to serialize NaN? (This would be a breaking change)
  • Allowing assigning null to a double? Which value would that be? (Confusing, and also breaking)
  • Adding parameters to somehow work around this, for instance like Python's json module is doing this (producing invalid JSON):

If allow_nan is false (default: True), then it will be a ValueError to serialize out of range float values (nan, inf, -inf) in strict compliance of the JSON specification. If allow_nan is true, their JavaScript equivalents (NaN, Infinity, -Infinity) will be used.

I don't really like any of these options. What do you think?

@nlohmann nlohmann added the state: please discuss please discuss the issue or vote for your favorite option label Jul 4, 2018
@giumas
Copy link
Author

giumas commented Jul 5, 2018

I see. What about something like this?

map<int64_t, double> reloaded_map = j2["data"].get<std::map<int64_t, double>>(std::nan(""));

That is, an optional parameter to be used as a value in case of null? What do you think about?

@nlohmann
Copy link
Owner

nlohmann commented Jul 9, 2018

I'm not sure if we can provide such an overload for all situations where the get function can be called - I mean, it may not be useful for other template arguments. I think for this special situation, a user-defined function may be more suitable.

@giumas
Copy link
Author

giumas commented Jul 11, 2018

Well, assuming that I well understood what you are proposing, a user-defined function would be more flexible too.

@nlohmann
Copy link
Owner

Great. Can I close the issue?

@giumas
Copy link
Author

giumas commented Jul 11, 2018

Now I am quite sure that I misunderstood your "user-defined function"!

If you meant that "the user will write some code to workaround the issue", how can I retrieve just the key of the pair in case that there is an exception in the value?

@nlohmann
Copy link
Owner

With "user-defined" function I meant that the problem is conceptually solvable with a function using only the public interface of the library. I write "conceptually", because you may not be able to write code like

map<int64_t, double> reloaded_map = j.get<std::map<int64_t, double>>();

but rather need to define a function like

void my_conversion(const json& j, std::map<int64_t, double>& dest)

in which you need to take care about only copying the non-null values.

For this, the items function should be helpful. It allows code like

#include <iostream>
#include "json.hpp"

using json = nlohmann::json;

void my_conversion(const json& j, std::map<std::string, double>& dest)
{
    for (auto& el: j.items())
    {
        if (el.value().is_number())
        {
            dest[el.key()] = el.value();
        }
    }
}

int main()
{
    json j = {{"foo", 43.12}, {"bar", nullptr}};
    std::map<std::string, double> m;
    
    my_conversion(j, m);
    std::cout << m.size() << std::endl;
}

@nlohmann nlohmann added solution: proposed fix a fix for the issue has been proposed and waits for confirmation and removed state: please discuss please discuss the issue or vote for your favorite option labels Jul 12, 2018
@giumas
Copy link
Author

giumas commented Jul 12, 2018

In my case, I have a std::map<int64_t, double>. I have modified my initial minimal example to follow your example, but it does not work as expected:

#include <iostream>
#include <iomanip>
#include <map>

#include "rawdata/ext/json/json.hpp"
#include <fstream>

using namespace std;
using json = nlohmann::json;

int main()
{
	// a map with a nan-value pair
	map<int64_t, double> test_map;
	test_map[123] = 1.2342;
	test_map[434] = std::nan("");
	test_map[999] = 243.12;

	// storage file
	string file_path = "test.json";

	// serialization
	json j;
	j["data"] = test_map;
	string json_str = j.dump(3);
	cout << "json str: " << json_str << endl;
	// write to disk
	std::ofstream ofs( file_path );
	if(!ofs)
	{
		cerr << "issue in opening " << file_path << endl;
		exit( -1 );
	}
	ofs << json_str;
	ofs.close();

	// de-serialization
	std::ifstream ifs( file_path );
	if(!ifs)
	{
		cerr << "issue in opening " << file_path << endl;
		exit( -1 );
	}
	json j2;
	ifs >> j2;
	ifs.close();
	try
	{
          map<int64_t, double> reloaded_map;
          for(auto& el : j2["data"].items())
          {
            if(el.value().is_number())
            {
              reloaded_map[std::stoll(el.key())] = el.value();
            }
            else
            {
              reloaded_map[std::stoll(el.key())] = std::nan("");
            }
          }
	  
          for(const auto& pair: reloaded_map)
	  {
		cout << "- " << pair.first << ": " << pair.second << endl;
	  }
	}
	catch(const std::exception& e)
	{
		cerr << "issue in parsing json: " << e.what() << endl;
	}
}

This is the output:

- 0: nan
- 1: nan
- 2: nan

What am I doing wrong?

@nlohmann
Copy link
Owner

When you map key type is not a string, the map will not be serialized to an object, because JSON requires string keys. Instead, the map is serialized to a list of pairs:

{"data":[[123,1.2342],[434,null],[999,243.12]]}

Then you can simply iterate the list of these pairs and check whether each second entry is a number:

#include <iostream>
#include "json.hpp"

using json = nlohmann::json;

int main()
{
    // a map with a nan-value pair
    std::map<int64_t, double> test_map;
    test_map[123] = 1.2342;
    test_map[434] = std::nan("");
    test_map[999] = 243.12;
    
    // serialization
    json j;
    j["data"] = test_map;
    
    // roundtrip
    json j2 = json::parse(j.dump());

    std::map<int64_t, double> reloaded_map;
    for (auto& entry : j2["data"])
    {
        assert(entry.is_array());
        assert(entry.size() == 2);
        if (entry[1].is_number())
        {
            reloaded_map[entry[0]] = entry[1];
        }
    }
    
    for (const auto& pair: reloaded_map)
    {
        std::cout << "- " << pair.first << ": " << pair.second << std::endl;
    }
}

@giumas
Copy link
Author

giumas commented Jul 13, 2018

It worked. Thank you!

@giumas giumas closed this as completed Jul 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

No branches or pull requests

2 participants