application developers must be aware that if they also link to nlohmann::json it must be the exact same version (even though the library is header-only); and
if we ever decide to switch to a different JSON library, it would require a large API change to VTK or supporting two JSON libraries at once (which we have had to deal with for XML).
Proposal
Our plan is provide a vtkJSON class in VTK::CommonCore that contains an internal (non-public) instance of an nlohmann::json object and some public methods/operators (.at(), [], .contains(), …) that it forwards to its internal member. Classes that currently use nlohmann::json directly will be modified to use vtkJSON instead. This will isolate nlohmann::json from the public API.
At the same time, we will also provide a “semi-public” API (with no guarantee of stability) to fetch the internal nlohmann::json instance from a vtkJSON instance. This utility may live in its own module and would introduce a public dependency on nlohmann::json as well as VTK::CommonCore.
Does anyone have input to provide before we pursue this change?
Sounds like the right solution, but I’m curious about whether vtkJSON is going to be a vtkObject or not. Anything that’s a vtkObject (or vtkObjectBase) is always handled as a pointer (or smart pointer) and things like operator[] make no sense for such objects. On the other hand, if vtkJSON does not derive from vtkObjectBase, then it can have operator[] (and the object and its operator can be wrapped in Python). Also I’m wondering if vtkJSON will contain a nlohmann::json object or if it will just contain a pointer to a nlohmann::json object.
Thank you, this sounds good to me. It makes sense for VTK to have an object (a proper, Python-wrapped VTK object) similarly to vtkXMLDataElement.
I would expect it to have VTK-style interface, for example myjson->GetProperty(name). But in addition to the VTK-style interface it could be OK to add STL-like interface as well, for convenience when using in C++.
We should avoid any nlohmann-specific, nlohmann-style API, because eventually we will probably have to switch to rapidjson in performance-critical parts of the code (its API is a pain, but it is about 10 times faster) and would be nice if we could still use the same API.
For future-proofing and avoid even VTK-internal dependencies to a specific JSON toolkit, it could make sense to specify an abstract vtkJSON class. This class could also contain some helper functions that are common across all JSON implementations. For now, we can do without this separate interface class, but at least make sure to declare all public methods as virtual to make it possible for VTK-based projects to add faster alternative implementations.
If this will not be a proper VTK object (and will not be Python-wrapped) then this class is basically just an internal VTK implementation detail. A good solution to fix the nlohmann-json interface leakage, but has no use beyond that, because the object and all methods that use it would be inaccessible from Python. Nowadays in most of our projects high-level logic is implemented in Python or at least all objects have to be accessible from Python for debugging and testing, we cannot rely on any non-Python-wrappable C++ code.
In this case, it would be better to remove the vtk prefix to avoid any confusion (and allow using vtkJSON in the future for a proper VTK object).
Note that wrappers might be able to do marshalling to more language-ergonomic representations (e.g., taking/returning a Python dictionary). I don’t think automatic wrapping would get anything useful enough to offset the vtkObjectBase drawbacks.
vtkObject or not
I don’t think vtkObject makes sense because it should probably just have value semantics rather than the refcounted-reference semantics basically required by vtkObject.
My head hurts when I try to think about how this would get misused (i.e., it would only ever work as a local variable because as a member “who owns the actual thing” sounds like a nightmare to reason about). But I also don’t think that only supporting bigobj[innerbigobj] as a full copy of the inner object makes sense either. This will need documentation, probably along the lines of “use auto when storing lookups, vtkJson in objects, and use vtkJsonRef at API boundaries” or something.
My thought was to mitigate this in cases where the JSON data being referenced was owned by a vtkObject subclass; we could have vtkJSONRef own a reference to the VTK object that owns the JSON data.
I agree we would still need to warn developers in cases where JSON data was not owned by a vtkObject:
You are responsible to ensure the underlying JSON object’s lifetime extends beyond that of every vtkJSONRef that references it.
vtkObject makes sense, because it is a really useful feature of VTK that 99.5% of objects work the same way (memory management by internal reference counting and many other conventions in naming, etc.). I would expect that the vtkJSON object to work the same way as vtkXMLDataElement: owning the document that it stores.
Having a json object in VTK would be awesome, but if you cannot make it a real vtkObject that follows all VTK conventions (internal reference counting, usual naming conventions, Python-wrapped, own the document inside same way as the XML object, etc.) then don’t make it look like a VTK object, just hide it, make it an internal implementation detail that does not show up anywhere for VTK users.
VTK also uses several classes that aren’t derived from vtkObjectBase. The wrapping of these classes is fully automatic, but is done in a slightly different manner than vtkObjectBase-derived classes. First, these classes have no New() method, and instead the public C++ constructors are wrapped to create an equivalent Python constructor. Second, the Python object contains its own copy of the C++ object, rather than containing just a pointer to the C++ object. The vast majority of these classes are lightweight containers and numerical types.
Here is a suggestion on how we might implement vtkJSON. If we add vtkJSONRef at the same time as vtkJSONRef, we can make some of the API function a bit more like nlohmann::json (specifically places like the [ ] operator where it could return a reference rather than a value).
// SPDX-FileCopyrightText: Copyright (c) Ken Martin, Will Schroeder, Bill Lorensen
// SPDX-License-Identifier: BSD-3-Clause
/**
* @class vtkJSON
* @brief An object holding parsed JSON data.
*
* vtkJSON is a facade for nlohmann::json in order to keep nlohmann::json
* out of VTK's public API. This will allow us to change the way JSON is
* represented in VTK without breaking changes to external users.
*
* If you must have access to the underlying nlohmann::json object,
* then use vtkJSONRaw, which is not part of VTK's public API.
* It is unsupported and subject to change, but will allow you to
* construct vtkJSON from an nlohmann::json instance, fetch an
* nlohmann::json instance, and provide other access.
*
* Classes in VTK that wish to return JSON data should return
* vtkJSON instances constructed via vtkJSONRaw's API.
*/
#ifndef vtkJSON_h
#define vtkJSON_h
#include "vtkSmartPointer.h" // For return types and export macro.
// #include "vtkAbstractArray.h" // For arguments/returns.
#include "vtkStringToken.h" // For GetType().
#include "vtkType.h" // For AsArray() default argument.
#include "vtk_nlohmannjson.h"
#include <iostream>
VTK_ABI_NAMESPACE_BEGIN
class vtkAbstractArray;
class vtkDataArray;
class VTKCOMMONCORE_EXPORT vtkJSON
{
public:
vtkJSON() = default;
virtual ~vtkJSON() = default;
vtkJSON(const vtkJSON&) = default;
vtkJSON& operator = (const vtkJSON&) = default;
vtkJSON(bool value) : Data(value) {}
vtkJSON(const std::string& value) : Data(value) {}
vtkJSON(std::int64_t value) : Data(value) {}
vtkJSON(std::uint64_t value) : Data(value) {}
vtkJSON(double value) : Data(value) {}
vtkJSON(vtkAbstractArray* value);
// Dump pretty-printed contents to \a os.
void PrintSelf(ostream& os = std::cout);
/// Convert internal JSON data into the given format.
///
/// This method will throw a std::logic_error if no conversion is possible.
template<typename T>
T get()
{
try
{
return this->Data.get<T>();
}
catch (nlohmann::json::exception& e)
{
throw std::logic_error(e.what());
}
}
vtkJSON contains(const std::string& key)
{
return this->Data.contains(key);
}
/// Return the value at a given \a key, throwing std::invalid_argument if
/// no \a key exists.
vtkJSON at(const std::string& key) const
{
try
{
return this->Data.at(key);
}
catch (nlohmann::json::exception& e)
{
throw std::invalid_argument(e.what());
}
}
/// Return the value at a given array \a index.
///
/// If this object does not hold an array, std::logic_error is thrown.
vtkJSON operator[] (std::size_t& index)
{
try
{
return this->Data[index];
}
catch (nlohmann::json::exception& e)
{
throw std::logic_error(e.what());
}
}
/// Return the value at a given dictionary \a key.
///
/// This method will throw std::logic_error if the object is not
/// a dictionary (i.e., it holds a number, string, or array).
vtkJSON operator[] (const std::string& key)
{
try
{
return this->Data[key];
}
catch (nlohmann::json::exception& e)
{
throw std::logic_error(e.what());
}
}
vtkStringToken type() const
{
using namespace vtk::literals;
switch (this->Data.type())
{
case nlohmann::json::value_t::null: return "null"_token;
case nlohmann::json::value_t::boolean: return "boolean"_token;
case nlohmann::json::value_t::string: return "string"_token;
case nlohmann::json::value_t::number_integer: return "signed integer"_token;
case nlohmann::json::value_t::number_unsigned: return "unsigned integer"_token;
case nlohmann::json::value_t::number_float: return "floating-point"_token;
case nlohmann::json::value_t::object: return "dictionary"_token;
case nlohmann::json::value_t::array: return "array"_token;
case nlohmann::json::value_t::binary: return "binary blob"_token;
case nlohmann::json::value_t::discarded: return "discarded"_token;
}
}
/// A python-wrappable method to convert the data to a boolean value.
bool AsBoolean() const
{
try
{
return this->Data.get<bool>();
}
catch (nlohmann::json::exception& e)
{
throw std::logic_error(e.what());
}
}
/// A python-wrappable method to convert the data to a string value.
std::string AsString() const
{
try
{
return this->Data.get<std::string>();
}
catch (nlohmann::json::exception& e)
{
throw std::logic_error(e.what());
}
}
/// A python-wrappable method to convert the data to a signed integer value.
std::int64_t AsSignedInteger() const
{
try
{
return this->Data.get<std::int64_t>();
}
catch (nlohmann::json::exception& e)
{
throw std::logic_error(e.what());
}
}
/// A python-wrappable method to convert the data to an unsigned integer value.
std::uint64_t AsUnsignedInteger() const
{
try
{
return this->Data.get<std::uint64_t>();
}
catch (nlohmann::json::exception& e)
{
throw std::logic_error(e.what());
}
}
/// A python-wrappable method to convert the data to a floating-point value.
double AsFloatingPoint() const
{
try
{
return this->Data.get<double>();
}
catch (nlohmann::json::exception& e)
{
throw std::logic_error(e.what());
}
}
/// A python-wrappable method to convert the data to a VTK array.
///
/// If this object cannot be interpreted as an array of the requested \a arrayType,
/// a std::logic_error will be thrown.
vtkSmartPointer<vtkAbstractArray> AsArray(int arrayType = VTK_VARIANT) const;
/// A python-wrappable method to convert the data to a VTK array.
///
/// If this object cannot be interpreted as an array of the requested \a arrayType,
/// the method will return false (but the contents of \a array may be altered).
/// This method will not throw exceptions.
bool CopyInto(vtkAbstractArray* array) const;
/// Construct a VTK array of the given \a ArrayType and populate it with this object's data.
///
/// This method may throw a std::logic_error if the JSON data cannot be converted.
template<typename ArrayType>
vtkSmartPointer<ArrayType> AsArrayOfType()
{
auto result = vtkSmartPointer<ArrayType>::New();
this->CopyInto(result);
return result;
}
/// A python-wrappable method to convert the data to a VTK data-array.
///
/// If this object cannot be interpreted as an array of the requested \a arrayType,
/// a std::logic_error will be thrown.
vtkSmartPointer<vtkDataArray> AsDataArray(int arrayType = VTK_DOUBLE) const;
/// Return the size of this data (the number of keys if a dictionary, the number of values if
/// an array, 0 for null, and 1 for plain-old-data types such as strings or numbers).
std::size_t size() const
{
return this->Data.size();
}
/// Return true if this data is null or an empty dictionary or array.
bool empty() const
{
return this->Data.empty();
}
/// Return a string token holding the type of JSON data.
vtkStringToken GetType() const { return this->type(); }
/// Return the size of this data (the number of keys if a dictionary, the number of values if
/// an array, 0 for null, and 1 for plain-old-data types such as strings or numbers).
std::size_t GetSize() const { return this->size(); }
/// Return true if this data is null or an empty dictionary or array.
bool IsEmpty() const { return this->empty(); }
/// Set a dictionary entry.
///
/// These implementations will change to handle exceptions.
void SetValue(const std::string& key, bool value) { this->Data[key] = value; }
void SetValue(const std::string& key, const std::string& value) { this->Data[key] = value; }
void SetValue(const std::string& key, std::uint64_t value) { this->Data[key] = value; }
void SetValue(const std::string& key, std::int64_t value) { this->Data[key] = value; }
void SetValue(const std::string& key, double value) { this->Data[key] = value; }
/// Set an array entry.
///
/// These implementations will change to handle exceptions.
void SetValue(std::size_t key, bool value) { this->Data[key] = value; }
void SetValue(std::size_t key, const std::string& value) { this->Data[key] = value; }
void SetValue(std::size_t key, std::uint64_t value) { this->Data[key] = value; }
void SetValue(std::size_t key, std::int64_t value) { this->Data[key] = value; }
void SetValue(std::size_t key, double value) { this->Data[key] = value; }
protected:
/// Construction from nlohmann::json objects is not part of the public API.
vtkJSON(const nlohmann::json& data)
: Data(data)
{
}
nlohmann::json Data;
};
class vtkJSONRaw : public vtkJSON
{
public:
vtkJSONRaw(vtkJSON&& value);
vtkJSONRaw(const vtkJSON& value) : Data(value.Data) {}
vtkJSONRaw(const nlohmann::json& value) : Data(value) {}
nlohmann::json& GetData() { return this->Data; }
const nlohmann::json& GetData() const { return this->Data; }
};
VTK_ABI_NAMESPACE_END
#endif
Please feel free to comment. Comments on the overall approach (allowing exceptions but converting them to std::exception; sticking to the nlohmann::json API but also adding VTK-style accessors) are more valuable at this point that nitpicks on the suggested implementation details as there will be a merge request for that.
@dcthomp,
We would like to avoid inclusion of third party headers in a VTK header that gets installed.
I see that it includes vtk_nlohmannjson.h in the header. It uses nlohmann::json Data under protected without actually including #include VTK_NLOHMANN_JSON(json.hpp).
It is unclear how you have access to nlohmann::json. Am I missing something?
It should probably have a private vtkInternals instance, which, in the vtkJSON.cxx file is declared with a nlohmann::json Data member. That C++ file can do #include VTK_NLOHMANN_JSON(json.hpp)
In vtkJSON.h
#ifndef vtkJSON_h
#define vtkJSON_h
#include "vtkCommonCoreModule.h" // for export macro
VTK_ABI_NAMESPACE_BEGIN
class VTKCOMMONCORE_EXPORT vtkJSON
{
public:
/// All API without nlohmann::json
private:
class vtkInternals;
std::unique_ptr<vtkInternals> Internals;
class vtkJSONRaw may need rethinking because we cannot have nlohmann::json in this header.
};
VTK_ABI_NAMESPACE_END
#endif
Templated get<> methods are unwrappable. VTK doesn’t have a templated get<> method for vtkVariant, either. Maybe it’s something that could be considered for the more distant future, but not for the initial implementation.
Ah, I didn’t scroll down far enough to see AsString() et al.
Mostly for the benefit of others reading this, the get<> declaration doesn’t need json.hpp, if its instantiations are declared extern. It’s a bit complicated but there’s precedent: a good current example in VTK is vtkValueFromString.h and vtkValueFromString.cxx.
If wrapping of get<>, is eventually implemented, would ideally be able to guess the best-fit template type, so that obj.get() would simply return the json value as a native Python value (looking far into the future).
